All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Wait until after wm optimization to drop runtime PM reference
@ 2016-03-04 23:59 Matt Roper
  2016-03-05  1:25 ` Imre Deak
                   ` (3 more replies)
  0 siblings, 4 replies; 16+ messages in thread
From: Matt Roper @ 2016-03-04 23:59 UTC (permalink / raw)
  To: intel-gfx

At the end of an atomic commit, we currently wait for vblanks to
complete, call put() on the various runtime PM references, and then try
to optimize our watermarks (on platforms that need two-step watermark
programming).  This can lead to watermark registers being programmed
while the power well is powered down.  We need to wait until after
watermark optimization is complete before dropping our runtime power
references.

Note that in the future the watermark optimization is probably going to
move to an asynchronous workqueue task that happens at some arbitrary
point after vblank.  When we make that change, we'll no longer
necessarily be operating under the power reference held here, so we'll
need to wrap the watermark register programmin in a call to
intel_runtime_pm_get_if_in_use() or similar.

Cc: arun.siluvery@linux.intel.com
Cc: ville.syrjala@linux.intel.com
Cc: maarten.lankhorst@linux.intel.com
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94349
Fixes: ed4a6a7ca853 ("drm/i915: Add two-stage ILK-style watermark programming (v11)")
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 62d36a7..0af08d7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13789,16 +13789,6 @@ static int intel_atomic_commit(struct drm_device *dev,
 	if (!state->legacy_cursor_update)
 		intel_atomic_wait_for_vblanks(dev, dev_priv, crtc_vblank_mask);
 
-	for_each_crtc_in_state(state, crtc, crtc_state, i) {
-		intel_post_plane_update(to_intel_crtc(crtc));
-
-		if (put_domains[i])
-			modeset_put_power_domains(dev_priv, put_domains[i]);
-	}
-
-	if (intel_state->modeset)
-		intel_display_power_put(dev_priv, POWER_DOMAIN_MODESET);
-
 	/*
 	 * Now that the vblank has passed, we can go ahead and program the
 	 * optimal watermarks on platforms that need two-step watermark
@@ -13813,6 +13803,16 @@ static int intel_atomic_commit(struct drm_device *dev,
 			dev_priv->display.optimize_watermarks(intel_cstate);
 	}
 
+	for_each_crtc_in_state(state, crtc, crtc_state, i) {
+		intel_post_plane_update(to_intel_crtc(crtc));
+
+		if (put_domains[i])
+			modeset_put_power_domains(dev_priv, put_domains[i]);
+	}
+
+	if (intel_state->modeset)
+		intel_display_power_put(dev_priv, POWER_DOMAIN_MODESET);
+
 	mutex_lock(&dev->struct_mutex);
 	drm_atomic_helper_cleanup_planes(dev, state);
 	mutex_unlock(&dev->struct_mutex);
-- 
2.1.4

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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH] drm/i915: Wait until after wm optimization to drop runtime PM reference
  2016-03-04 23:59 [PATCH] drm/i915: Wait until after wm optimization to drop runtime PM reference Matt Roper
@ 2016-03-05  1:25 ` Imre Deak
  2016-03-07 16:10   ` Matt Roper
  2016-03-07 11:47 ` ✗ Fi.CI.BAT: failure for " Patchwork
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 16+ messages in thread
From: Imre Deak @ 2016-03-05  1:25 UTC (permalink / raw)
  To: Matt Roper, intel-gfx

Hi Matt,

On Fri, 2016-03-04 at 15:59 -0800, Matt Roper wrote:
> At the end of an atomic commit, we currently wait for vblanks to
> complete, call put() on the various runtime PM references, and then
> try
> to optimize our watermarks (on platforms that need two-step watermark
> programming).  This can lead to watermark registers being programmed
> while the power well is powered down.  We need to wait until after
> watermark optimization is complete before dropping our runtime power
> references.
> 
> Note that in the future the watermark optimization is probably going
> to
> move to an asynchronous workqueue task that happens at some arbitrary
> point after vblank.  When we make that change, we'll no longer
> necessarily be operating under the power reference held here, so
> we'll
> need to wrap the watermark register programmin in a call to
> intel_runtime_pm_get_if_in_use() or similar.
> 
> Cc: arun.siluvery@linux.intel.com
> Cc: ville.syrjala@linux.intel.com
> Cc: maarten.lankhorst@linux.intel.com
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94349
> Fixes: ed4a6a7ca853 ("drm/i915: Add two-stage ILK-style watermark
> programming (v11)")
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 62d36a7..0af08d7 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13789,16 +13789,6 @@ static int intel_atomic_commit(struct
> drm_device *dev,
>  	if (!state->legacy_cursor_update)
>  		intel_atomic_wait_for_vblanks(dev, dev_priv,
> crtc_vblank_mask);
>  
> -	for_each_crtc_in_state(state, crtc, crtc_state, i) {
> -		intel_post_plane_update(to_intel_crtc(crtc));
> -
> -		if (put_domains[i])
> -			modeset_put_power_domains(dev_priv,
> put_domains[i]);
> -	}
> -
> -	if (intel_state->modeset)
> -		intel_display_power_put(dev_priv,
> POWER_DOMAIN_MODESET);
> -
>  	/*
>  	 * Now that the vblank has passed, we can go ahead and
> program the
>  	 * optimal watermarks on platforms that need two-step
> watermark
> @@ -13813,6 +13803,16 @@ static int intel_atomic_commit(struct
> drm_device *dev,
>  			dev_priv-
> >display.optimize_watermarks(intel_cstate);
>  	}
>  
> +	for_each_crtc_in_state(state, crtc, crtc_state, i) {
> +		intel_post_plane_update(to_intel_crtc(crtc));
> +
> +		if (put_domains[i])
> +			modeset_put_power_domains(dev_priv,
> put_domains[i]);
> +	}
> +
> +	if (intel_state->modeset)
> +> 		intel_display_power_put(dev_priv, POWER_DOMAIN_MODESET);
> +

Since the error was caused by writing some WM register while the HW is
suspended, I don't see what would be the point of programming it if the
register loses its value anyway after dropping the power reference.
What would make sense to me is to avoid programming the WM registers if
all the outputs are disabled (like they were when the bug triggered)
and program them next time around when any output gets enabled.

--Imre


>  	mutex_lock(&dev->struct_mutex);
>  	drm_atomic_helper_cleanup_planes(dev, state);
>  	mutex_unlock(&dev->struct_mutex);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* ✗ Fi.CI.BAT: failure for drm/i915: Wait until after wm optimization to drop runtime PM reference
  2016-03-04 23:59 [PATCH] drm/i915: Wait until after wm optimization to drop runtime PM reference Matt Roper
  2016-03-05  1:25 ` Imre Deak
@ 2016-03-07 11:47 ` Patchwork
  2016-03-08 17:26   ` Matt Roper
  2016-03-07 11:53 ` [PATCH] " Maarten Lankhorst
  2016-03-22 11:04 ` Ville Syrjälä
  3 siblings, 1 reply; 16+ messages in thread
From: Patchwork @ 2016-03-07 11:47 UTC (permalink / raw)
  To: Matt Roper; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Wait until after wm optimization to drop runtime PM reference
URL   : https://patchwork.freedesktop.org/series/4135/
State : failure

== Summary ==

Series 4135v1 drm/i915: Wait until after wm optimization to drop runtime PM reference
http://patchwork.freedesktop.org/api/1.0/series/4135/revisions/1/mbox/

Test kms_flip:
        Subgroup basic-flip-vs-wf_vblank:
                fail       -> PASS       (bsw-nuc-2)
        Subgroup basic-plain-flip:
                dmesg-warn -> PASS       (hsw-gt2)
Test kms_force_connector_basic:
        Subgroup force-load-detect:
                pass       -> SKIP       (ivb-t430s)
Test kms_pipe_crc_basic:
        Subgroup read-crc-pipe-c:
                dmesg-warn -> PASS       (hsw-gt2)
        Subgroup suspend-read-crc-pipe-a:
                dmesg-warn -> PASS       (skl-i5k-2)
                skip       -> PASS       (hsw-brixbox)
        Subgroup suspend-read-crc-pipe-c:
                pass       -> DMESG-WARN (bsw-nuc-2)
                pass       -> INCOMPLETE (hsw-gt2)
Test pm_rpm:
        Subgroup basic-pci-d3-state:
                dmesg-warn -> PASS       (snb-dellxps)
        Subgroup basic-rte:
                pass       -> DMESG-WARN (bsw-nuc-2)

bdw-nuci7        total:183  pass:172  dwarn:0   dfail:0   fail:0   skip:11 
bdw-ultra        total:183  pass:165  dwarn:0   dfail:0   fail:0   skip:18 
bsw-nuc-2        total:183  pass:147  dwarn:2   dfail:0   fail:0   skip:34 
byt-nuc          total:183  pass:152  dwarn:0   dfail:0   fail:0   skip:31 
hsw-brixbox      total:183  pass:164  dwarn:0   dfail:0   fail:0   skip:19 
hsw-gt2          total:119  pass:109  dwarn:0   dfail:0   fail:0   skip:9  
ilk-hp8440p      total:183  pass:125  dwarn:0   dfail:0   fail:0   skip:58 
ivb-t430s        total:183  pass:161  dwarn:0   dfail:0   fail:0   skip:22 
skl-i5k-2        total:183  pass:163  dwarn:0   dfail:0   fail:0   skip:20 
skl-i7k-2        total:183  pass:163  dwarn:0   dfail:0   fail:0   skip:20 
snb-dellxps      total:183  pass:154  dwarn:0   dfail:0   fail:0   skip:29 

Results at /archive/results/CI_IGT_test/Patchwork_1532/

d369e0096716c6000139162b3b340f684f0a51da drm-intel-nightly: 2016y-03m-04d-17h-18m-08s UTC integration manifest
b0744e82af66e41a8dca48bfa6ff8f38e1d9454f drm/i915: Wait until after wm optimization to drop runtime PM reference

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] drm/i915: Wait until after wm optimization to drop runtime PM reference
  2016-03-04 23:59 [PATCH] drm/i915: Wait until after wm optimization to drop runtime PM reference Matt Roper
  2016-03-05  1:25 ` Imre Deak
  2016-03-07 11:47 ` ✗ Fi.CI.BAT: failure for " Patchwork
@ 2016-03-07 11:53 ` Maarten Lankhorst
  2016-03-07 16:26   ` Matt Roper
  2016-03-22 11:04 ` Ville Syrjälä
  3 siblings, 1 reply; 16+ messages in thread
From: Maarten Lankhorst @ 2016-03-07 11:53 UTC (permalink / raw)
  To: Matt Roper, intel-gfx

Op 05-03-16 om 00:59 schreef Matt Roper:
> At the end of an atomic commit, we currently wait for vblanks to
> complete, call put() on the various runtime PM references, and then try
> to optimize our watermarks (on platforms that need two-step watermark
> programming).  This can lead to watermark registers being programmed
> while the power well is powered down.  We need to wait until after
> watermark optimization is complete before dropping our runtime power
> references.
>
> Note that in the future the watermark optimization is probably going to
> move to an asynchronous workqueue task that happens at some arbitrary
> point after vblank.  When we make that change, we'll no longer
> necessarily be operating under the power reference held here, so we'll
> need to wrap the watermark register programmin in a call to
> intel_runtime_pm_get_if_in_use() or similar.
>
> Cc: arun.siluvery@linux.intel.com
> Cc: ville.syrjala@linux.intel.com
> Cc: maarten.lankhorst@linux.intel.com
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94349
> Fixes: ed4a6a7ca853 ("drm/i915: Add two-stage ILK-style watermark programming (v11)")
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
post_plane_update can call intel_update_watermarks, will this cause any unintended behavioral changes
if intel_update_watermarks is called before optimize_watermarks?

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] drm/i915: Wait until after wm optimization to drop runtime PM reference
  2016-03-05  1:25 ` Imre Deak
@ 2016-03-07 16:10   ` Matt Roper
  2016-03-07 16:28     ` Imre Deak
  0 siblings, 1 reply; 16+ messages in thread
From: Matt Roper @ 2016-03-07 16:10 UTC (permalink / raw)
  To: Imre Deak; +Cc: intel-gfx

On Sat, Mar 05, 2016 at 03:25:05AM +0200, Imre Deak wrote:
> Hi Matt,
> 
> On Fri, 2016-03-04 at 15:59 -0800, Matt Roper wrote:
> > At the end of an atomic commit, we currently wait for vblanks to
> > complete, call put() on the various runtime PM references, and then
> > try
> > to optimize our watermarks (on platforms that need two-step watermark
> > programming).  This can lead to watermark registers being programmed
> > while the power well is powered down.  We need to wait until after
> > watermark optimization is complete before dropping our runtime power
> > references.
> > 
> > Note that in the future the watermark optimization is probably going
> > to
> > move to an asynchronous workqueue task that happens at some arbitrary
> > point after vblank.  When we make that change, we'll no longer
> > necessarily be operating under the power reference held here, so
> > we'll
> > need to wrap the watermark register programmin in a call to
> > intel_runtime_pm_get_if_in_use() or similar.
> > 
> > Cc: arun.siluvery@linux.intel.com
> > Cc: ville.syrjala@linux.intel.com
> > Cc: maarten.lankhorst@linux.intel.com
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94349
> > Fixes: ed4a6a7ca853 ("drm/i915: Add two-stage ILK-style watermark
> > programming (v11)")
> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 20 ++++++++++----------
> >  1 file changed, 10 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 62d36a7..0af08d7 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -13789,16 +13789,6 @@ static int intel_atomic_commit(struct
> > drm_device *dev,
> >  	if (!state->legacy_cursor_update)
> >  		intel_atomic_wait_for_vblanks(dev, dev_priv,
> > crtc_vblank_mask);
> >  
> > -	for_each_crtc_in_state(state, crtc, crtc_state, i) {
> > -		intel_post_plane_update(to_intel_crtc(crtc));
> > -
> > -		if (put_domains[i])
> > -			modeset_put_power_domains(dev_priv,
> > put_domains[i]);
> > -	}
> > -
> > -	if (intel_state->modeset)
> > -		intel_display_power_put(dev_priv,
> > POWER_DOMAIN_MODESET);
> > -
> >  	/*
> >  	 * Now that the vblank has passed, we can go ahead and
> > program the
> >  	 * optimal watermarks on platforms that need two-step
> > watermark
> > @@ -13813,6 +13803,16 @@ static int intel_atomic_commit(struct
> > drm_device *dev,
> >  			dev_priv-
> > >display.optimize_watermarks(intel_cstate);
> >  	}
> >  
> > +	for_each_crtc_in_state(state, crtc, crtc_state, i) {
> > +		intel_post_plane_update(to_intel_crtc(crtc));
> > +
> > +		if (put_domains[i])
> > +			modeset_put_power_domains(dev_priv,
> > put_domains[i]);
> > +	}
> > +
> > +	if (intel_state->modeset)
> > +> 		intel_display_power_put(dev_priv, POWER_DOMAIN_MODESET);
> > +
> 
> Since the error was caused by writing some WM register while the HW is
> suspended, I don't see what would be the point of programming it if the
> register loses its value anyway after dropping the power reference.
> What would make sense to me is to avoid programming the WM registers if
> all the outputs are disabled (like they were when the bug triggered)
> and program them next time around when any output gets enabled.

Yeah, that was actually the first approach I went with:

        https://patchwork.freedesktop.org/patch/75772/

Ville felt like it was more of a workaround than a fix though, so I
posted this alternative instead.


Matt


> 
> --Imre
> 
> 
> >  	mutex_lock(&dev->struct_mutex);
> >  	drm_atomic_helper_cleanup_planes(dev, state);
> >  	mutex_unlock(&dev->struct_mutex);

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] drm/i915: Wait until after wm optimization to drop runtime PM reference
  2016-03-07 11:53 ` [PATCH] " Maarten Lankhorst
@ 2016-03-07 16:26   ` Matt Roper
  0 siblings, 0 replies; 16+ messages in thread
From: Matt Roper @ 2016-03-07 16:26 UTC (permalink / raw)
  To: Maarten Lankhorst; +Cc: intel-gfx

On Mon, Mar 07, 2016 at 12:53:38PM +0100, Maarten Lankhorst wrote:
> Op 05-03-16 om 00:59 schreef Matt Roper:
> > At the end of an atomic commit, we currently wait for vblanks to
> > complete, call put() on the various runtime PM references, and then try
> > to optimize our watermarks (on platforms that need two-step watermark
> > programming).  This can lead to watermark registers being programmed
> > while the power well is powered down.  We need to wait until after
> > watermark optimization is complete before dropping our runtime power
> > references.
> >
> > Note that in the future the watermark optimization is probably going to
> > move to an asynchronous workqueue task that happens at some arbitrary
> > point after vblank.  When we make that change, we'll no longer
> > necessarily be operating under the power reference held here, so we'll
> > need to wrap the watermark register programmin in a call to
> > intel_runtime_pm_get_if_in_use() or similar.
> >
> > Cc: arun.siluvery@linux.intel.com
> > Cc: ville.syrjala@linux.intel.com
> > Cc: maarten.lankhorst@linux.intel.com
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94349
> > Fixes: ed4a6a7ca853 ("drm/i915: Add two-stage ILK-style watermark programming (v11)")
> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> post_plane_update can call intel_update_watermarks, will this cause any unintended behavioral changes
> if intel_update_watermarks is called before optimize_watermarks?

It shouldn't; intel_update_watermarks is the legacy watermark
programming function.  Any platform that that's been converted to atomic
shouldn't have a dev_priv->display.update_wm vfunc, so
intel_update_watermarks will be a noop.


Matt

> 
> ~Maarten

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] drm/i915: Wait until after wm optimization to drop runtime PM reference
  2016-03-07 16:10   ` Matt Roper
@ 2016-03-07 16:28     ` Imre Deak
  0 siblings, 0 replies; 16+ messages in thread
From: Imre Deak @ 2016-03-07 16:28 UTC (permalink / raw)
  To: Matt Roper; +Cc: intel-gfx

On ma, 2016-03-07 at 08:10 -0800, Matt Roper wrote:
> On Sat, Mar 05, 2016 at 03:25:05AM +0200, Imre Deak wrote:
> > Hi Matt,
> > 
> > On Fri, 2016-03-04 at 15:59 -0800, Matt Roper wrote:
> > > At the end of an atomic commit, we currently wait for vblanks to
> > > complete, call put() on the various runtime PM references, and then
> > > try
> > > to optimize our watermarks (on platforms that need two-step watermark
> > > programming).  This can lead to watermark registers being programmed
> > > while the power well is powered down.  We need to wait until after
> > > watermark optimization is complete before dropping our runtime power
> > > references.
> > > 
> > > Note that in the future the watermark optimization is probably going
> > > to
> > > move to an asynchronous workqueue task that happens at some arbitrary
> > > point after vblank.  When we make that change, we'll no longer
> > > necessarily be operating under the power reference held here, so
> > > we'll
> > > need to wrap the watermark register programmin in a call to
> > > intel_runtime_pm_get_if_in_use() or similar.
> > > 
> > > Cc: arun.siluvery@linux.intel.com
> > > Cc: ville.syrjala@linux.intel.com
> > > Cc: maarten.lankhorst@linux.intel.com
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94349
> > > Fixes: ed4a6a7ca853 ("drm/i915: Add two-stage ILK-style watermark
> > > programming (v11)")
> > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_display.c | 20 ++++++++++----------
> > >  1 file changed, 10 insertions(+), 10 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > > b/drivers/gpu/drm/i915/intel_display.c
> > > index 62d36a7..0af08d7 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -13789,16 +13789,6 @@ static int intel_atomic_commit(struct
> > > drm_device *dev,
> > >  	if (!state->legacy_cursor_update)
> > >  		intel_atomic_wait_for_vblanks(dev, dev_priv,
> > > crtc_vblank_mask);
> > >  
> > > -	for_each_crtc_in_state(state, crtc, crtc_state, i) {
> > > -		intel_post_plane_update(to_intel_crtc(crtc));
> > > -
> > > -		if (put_domains[i])
> > > -			modeset_put_power_domains(dev_priv,
> > > put_domains[i]);
> > > -	}
> > > -
> > > -	if (intel_state->modeset)
> > > -		intel_display_power_put(dev_priv,
> > > POWER_DOMAIN_MODESET);
> > > -
> > >  	/*
> > >  	 * Now that the vblank has passed, we can go ahead and
> > > program the
> > >  	 * optimal watermarks on platforms that need two-step
> > > watermark
> > > @@ -13813,6 +13803,16 @@ static int intel_atomic_commit(struct
> > > drm_device *dev,
> > >  			dev_priv-
> > > > display.optimize_watermarks(intel_cstate);
> > >  	}
> > >  
> > > +	for_each_crtc_in_state(state, crtc, crtc_state, i) {
> > > +		intel_post_plane_update(to_intel_crtc(crtc));
> > > +
> > > +		if (put_domains[i])
> > > +			modeset_put_power_domains(dev_priv,
> > > put_domains[i]);
> > > +	}
> > > +
> > > +	if (intel_state->modeset)
> > > +> 		intel_display_power_put(dev_priv, POWER_DOMAIN_MODESET);
> > > +
> > 
> > Since the error was caused by writing some WM register while the HW is
> > suspended, I don't see what would be the point of programming it if the
> > register loses its value anyway after dropping the power reference.
> > What would make sense to me is to avoid programming the WM registers if
> > all the outputs are disabled (like they were when the bug triggered)
> > and program them next time around when any output gets enabled.
> 
> Yeah, that was actually the first approach I went with:
> 
>         https://patchwork.freedesktop.org/patch/75772/
> 
> Ville felt like it was more of a workaround than a fix though, so I
> posted this alternative instead.

Ok, I haven't noticed that patch.

One other thing that I realized only after sending my email is that -
on some old platforms at least - it would still make sense to program
the WM state even with all outputs disabled: the description of
FW_BLC_Self[15] on GEN3 for example could mean that memory self refresh
is disabled whenever we set this bit to 0, independently of the display
output state, although this description is somewhat vague.

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: ✗ Fi.CI.BAT: failure for drm/i915: Wait until after wm optimization to drop runtime PM reference
  2016-03-07 11:47 ` ✗ Fi.CI.BAT: failure for " Patchwork
@ 2016-03-08 17:26   ` Matt Roper
  2016-03-22 12:55     ` Imre Deak
  0 siblings, 1 reply; 16+ messages in thread
From: Matt Roper @ 2016-03-08 17:26 UTC (permalink / raw)
  To: intel-gfx

On Mon, Mar 07, 2016 at 11:47:51AM +0000, Patchwork wrote:
> == Series Details ==
> 
> Series: drm/i915: Wait until after wm optimization to drop runtime PM reference
> URL   : https://patchwork.freedesktop.org/series/4135/
> State : failure
> 
> == Summary ==
> 
> Series 4135v1 drm/i915: Wait until after wm optimization to drop runtime PM reference
> http://patchwork.freedesktop.org/api/1.0/series/4135/revisions/1/mbox/
> 
> Test kms_flip:
>         Subgroup basic-flip-vs-wf_vblank:
>                 fail       -> PASS       (bsw-nuc-2)
>         Subgroup basic-plain-flip:
>                 dmesg-warn -> PASS       (hsw-gt2)
> Test kms_force_connector_basic:
>         Subgroup force-load-detect:
>                 pass       -> SKIP       (ivb-t430s)

https://bugs.freedesktop.org/show_bug.cgi?id=93769

> Test kms_pipe_crc_basic:
>         Subgroup read-crc-pipe-c:
>                 dmesg-warn -> PASS       (hsw-gt2)
>         Subgroup suspend-read-crc-pipe-a:
>                 dmesg-warn -> PASS       (skl-i5k-2)
>                 skip       -> PASS       (hsw-brixbox)
>         Subgroup suspend-read-crc-pipe-c:
>                 pass       -> DMESG-WARN (bsw-nuc-2)

https://bugs.freedesktop.org/show_bug.cgi?id=93294

>                 pass       -> INCOMPLETE (hsw-gt2)

Seems like the machine just died completely?  No
stdout/stderr/command/dmesg output available from CI and time is given
as 0:00:00.  Doesn't seem like it could be related to this patch.


> Test pm_rpm:
>         Subgroup basic-pci-d3-state:
>                 dmesg-warn -> PASS       (snb-dellxps)
>         Subgroup basic-rte:
>                 pass       -> DMESG-WARN (bsw-nuc-2)

https://bugs.freedesktop.org/show_bug.cgi?id=94164


Matt

> 
> bdw-nuci7        total:183  pass:172  dwarn:0   dfail:0   fail:0   skip:11 
> bdw-ultra        total:183  pass:165  dwarn:0   dfail:0   fail:0   skip:18 
> bsw-nuc-2        total:183  pass:147  dwarn:2   dfail:0   fail:0   skip:34 
> byt-nuc          total:183  pass:152  dwarn:0   dfail:0   fail:0   skip:31 
> hsw-brixbox      total:183  pass:164  dwarn:0   dfail:0   fail:0   skip:19 
> hsw-gt2          total:119  pass:109  dwarn:0   dfail:0   fail:0   skip:9  
> ilk-hp8440p      total:183  pass:125  dwarn:0   dfail:0   fail:0   skip:58 
> ivb-t430s        total:183  pass:161  dwarn:0   dfail:0   fail:0   skip:22 
> skl-i5k-2        total:183  pass:163  dwarn:0   dfail:0   fail:0   skip:20 
> skl-i7k-2        total:183  pass:163  dwarn:0   dfail:0   fail:0   skip:20 
> snb-dellxps      total:183  pass:154  dwarn:0   dfail:0   fail:0   skip:29 
> 
> Results at /archive/results/CI_IGT_test/Patchwork_1532/
> 
> d369e0096716c6000139162b3b340f684f0a51da drm-intel-nightly: 2016y-03m-04d-17h-18m-08s UTC integration manifest
> b0744e82af66e41a8dca48bfa6ff8f38e1d9454f drm/i915: Wait until after wm optimization to drop runtime PM reference
> 

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] drm/i915: Wait until after wm optimization to drop runtime PM reference
  2016-03-04 23:59 [PATCH] drm/i915: Wait until after wm optimization to drop runtime PM reference Matt Roper
                   ` (2 preceding siblings ...)
  2016-03-07 11:53 ` [PATCH] " Maarten Lankhorst
@ 2016-03-22 11:04 ` Ville Syrjälä
  3 siblings, 0 replies; 16+ messages in thread
From: Ville Syrjälä @ 2016-03-22 11:04 UTC (permalink / raw)
  To: Matt Roper; +Cc: intel-gfx

On Fri, Mar 04, 2016 at 03:59:39PM -0800, Matt Roper wrote:
> At the end of an atomic commit, we currently wait for vblanks to
> complete, call put() on the various runtime PM references, and then try
> to optimize our watermarks (on platforms that need two-step watermark
> programming).  This can lead to watermark registers being programmed
> while the power well is powered down.  We need to wait until after
> watermark optimization is complete before dropping our runtime power
> references.
> 
> Note that in the future the watermark optimization is probably going to
> move to an asynchronous workqueue task that happens at some arbitrary
> point after vblank.  When we make that change, we'll no longer
> necessarily be operating under the power reference held here, so we'll
> need to wrap the watermark register programmin in a call to
> intel_runtime_pm_get_if_in_use() or similar.
> 
> Cc: arun.siluvery@linux.intel.com
> Cc: ville.syrjala@linux.intel.com
> Cc: maarten.lankhorst@linux.intel.com
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94349
> Fixes: ed4a6a7ca853 ("drm/i915: Add two-stage ILK-style watermark programming (v11)")
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_display.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 62d36a7..0af08d7 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13789,16 +13789,6 @@ static int intel_atomic_commit(struct drm_device *dev,
>  	if (!state->legacy_cursor_update)
>  		intel_atomic_wait_for_vblanks(dev, dev_priv, crtc_vblank_mask);
>  
> -	for_each_crtc_in_state(state, crtc, crtc_state, i) {
> -		intel_post_plane_update(to_intel_crtc(crtc));
> -
> -		if (put_domains[i])
> -			modeset_put_power_domains(dev_priv, put_domains[i]);
> -	}
> -
> -	if (intel_state->modeset)
> -		intel_display_power_put(dev_priv, POWER_DOMAIN_MODESET);
> -
>  	/*
>  	 * Now that the vblank has passed, we can go ahead and program the
>  	 * optimal watermarks on platforms that need two-step watermark
> @@ -13813,6 +13803,16 @@ static int intel_atomic_commit(struct drm_device *dev,
>  			dev_priv->display.optimize_watermarks(intel_cstate);
>  	}
>  
> +	for_each_crtc_in_state(state, crtc, crtc_state, i) {
> +		intel_post_plane_update(to_intel_crtc(crtc));
> +
> +		if (put_domains[i])
> +			modeset_put_power_domains(dev_priv, put_domains[i]);
> +	}
> +
> +	if (intel_state->modeset)
> +		intel_display_power_put(dev_priv, POWER_DOMAIN_MODESET);
> +
>  	mutex_lock(&dev->struct_mutex);
>  	drm_atomic_helper_cleanup_planes(dev, state);
>  	mutex_unlock(&dev->struct_mutex);
> -- 
> 2.1.4

-- 
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] 16+ messages in thread

* Re: ✗ Fi.CI.BAT: failure for drm/i915: Wait until after wm optimization to drop runtime PM reference
  2016-03-08 17:26   ` Matt Roper
@ 2016-03-22 12:55     ` Imre Deak
  2016-03-22 13:51       ` Jani Nikula
  0 siblings, 1 reply; 16+ messages in thread
From: Imre Deak @ 2016-03-22 12:55 UTC (permalink / raw)
  To: Matt Roper, intel-gfx, Ville Syrjälä, Jani Nikula

On ti, 2016-03-08 at 09:26 -0800, Matt Roper wrote:
> On Mon, Mar 07, 2016 at 11:47:51AM +0000, Patchwork wrote:
> > == Series Details ==
> > 
> > Series: drm/i915: Wait until after wm optimization to drop runtime
> > PM reference
> > URL   : https://patchwork.freedesktop.org/series/4135/
> > State : failure
> > 
> > == Summary ==
> > 
> > Series 4135v1 drm/i915: Wait until after wm optimization to drop
> > runtime PM reference
> > http://patchwork.freedesktop.org/api/1.0/series/4135/revisions/1/mb
> > ox/
> > 
> > Test kms_flip:
> >         Subgroup basic-flip-vs-wf_vblank:
> >                 fail       -> PASS       (bsw-nuc-2)
> >         Subgroup basic-plain-flip:
> >                 dmesg-warn -> PASS       (hsw-gt2)
> > Test kms_force_connector_basic:
> >         Subgroup force-load-detect:
> >                 pass       -> SKIP       (ivb-t430s)
> 
> https://bugs.freedesktop.org/show_bug.cgi?id=93769
> 
> > Test kms_pipe_crc_basic:
> >         Subgroup read-crc-pipe-c:
> >                 dmesg-warn -> PASS       (hsw-gt2)
> >         Subgroup suspend-read-crc-pipe-a:
> >                 dmesg-warn -> PASS       (skl-i5k-2)
> >                 skip       -> PASS       (hsw-brixbox)
> >         Subgroup suspend-read-crc-pipe-c:
> >                 pass       -> DMESG-WARN (bsw-nuc-2)
> 
> https://bugs.freedesktop.org/show_bug.cgi?id=93294
> 
> >                 pass       -> INCOMPLETE (hsw-gt2)
> 
> Seems like the machine just died completely?  No
> stdout/stderr/command/dmesg output available from CI and time is
> given
> as 0:00:00.  Doesn't seem like it could be related to this patch.
> 
> 
> > Test pm_rpm:
> >         Subgroup basic-pci-d3-state:
> >                 dmesg-warn -> PASS       (snb-dellxps)
> >         Subgroup basic-rte:
> >                 pass       -> DMESG-WARN (bsw-nuc-2)
> 
> https://bugs.freedesktop.org/show_bug.cgi?id=94164
> 
> 
> Matt

Pushed to -dinq, thanks for the patch and the review.

I had to rebase it on top of Ville's recent
s/crtc_state/old_crtc_state/ change, please double check it.

Jani, this is for -fixes.

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: ✗ Fi.CI.BAT: failure for drm/i915: Wait until after wm optimization to drop runtime PM reference
  2016-03-22 12:55     ` Imre Deak
@ 2016-03-22 13:51       ` Jani Nikula
  2016-03-22 14:17         ` Imre Deak
  0 siblings, 1 reply; 16+ messages in thread
From: Jani Nikula @ 2016-03-22 13:51 UTC (permalink / raw)
  To: imre.deak, Matt Roper, intel-gfx, Ville Syrjälä

On Tue, 22 Mar 2016, Imre Deak <imre.deak@intel.com> wrote:
> [ text/plain ]
> On ti, 2016-03-08 at 09:26 -0800, Matt Roper wrote:
>> On Mon, Mar 07, 2016 at 11:47:51AM +0000, Patchwork wrote:
>> > == Series Details ==
>> > 
>> > Series: drm/i915: Wait until after wm optimization to drop runtime
>> > PM reference
>> > URL   : https://patchwork.freedesktop.org/series/4135/
>> > State : failure
>> > 
>> > == Summary ==
>> > 
>> > Series 4135v1 drm/i915: Wait until after wm optimization to drop
>> > runtime PM reference
>> > http://patchwork.freedesktop.org/api/1.0/series/4135/revisions/1/mb
>> > ox/
>> > 
>> > Test kms_flip:
>> >         Subgroup basic-flip-vs-wf_vblank:
>> >                 fail       -> PASS       (bsw-nuc-2)
>> >         Subgroup basic-plain-flip:
>> >                 dmesg-warn -> PASS       (hsw-gt2)
>> > Test kms_force_connector_basic:
>> >         Subgroup force-load-detect:
>> >                 pass       -> SKIP       (ivb-t430s)
>> 
>> https://bugs.freedesktop.org/show_bug.cgi?id=93769
>> 
>> > Test kms_pipe_crc_basic:
>> >         Subgroup read-crc-pipe-c:
>> >                 dmesg-warn -> PASS       (hsw-gt2)
>> >         Subgroup suspend-read-crc-pipe-a:
>> >                 dmesg-warn -> PASS       (skl-i5k-2)
>> >                 skip       -> PASS       (hsw-brixbox)
>> >         Subgroup suspend-read-crc-pipe-c:
>> >                 pass       -> DMESG-WARN (bsw-nuc-2)
>> 
>> https://bugs.freedesktop.org/show_bug.cgi?id=93294
>> 
>> >                 pass       -> INCOMPLETE (hsw-gt2)
>> 
>> Seems like the machine just died completely?  No
>> stdout/stderr/command/dmesg output available from CI and time is
>> given
>> as 0:00:00.  Doesn't seem like it could be related to this patch.
>> 
>> 
>> > Test pm_rpm:
>> >         Subgroup basic-pci-d3-state:
>> >                 dmesg-warn -> PASS       (snb-dellxps)
>> >         Subgroup basic-rte:
>> >                 pass       -> DMESG-WARN (bsw-nuc-2)
>> 
>> https://bugs.freedesktop.org/show_bug.cgi?id=94164
>> 
>> 
>> Matt
>
> Pushed to -dinq, thanks for the patch and the review.
>
> I had to rebase it on top of Ville's recent
> s/crtc_state/old_crtc_state/ change, please double check it.
>
> Jani, this is for -fixes.

Surely you added either

Cc: drm-intel-fixes@lists.freedesktop.org

or

Cc: stable@vger.kernel.org

in the commit when you pushed, then?

BR,
Jani.

-- 
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] 16+ messages in thread

* Re: ✗ Fi.CI.BAT: failure for drm/i915: Wait until after wm optimization to drop runtime PM reference
  2016-03-22 13:51       ` Jani Nikula
@ 2016-03-22 14:17         ` Imre Deak
  2016-03-22 15:56           ` Matt Roper
  0 siblings, 1 reply; 16+ messages in thread
From: Imre Deak @ 2016-03-22 14:17 UTC (permalink / raw)
  To: Jani Nikula, Matt Roper, intel-gfx, Ville Syrjälä

On ti, 2016-03-22 at 15:51 +0200, Jani Nikula wrote:
> On Tue, 22 Mar 2016, Imre Deak <imre.deak@intel.com> wrote:
> > [ text/plain ]
> > On ti, 2016-03-08 at 09:26 -0800, Matt Roper wrote:
> > > On Mon, Mar 07, 2016 at 11:47:51AM +0000, Patchwork wrote:
> > > > == Series Details ==
> > > > 
> > > > Series: drm/i915: Wait until after wm optimization to drop
> > > > runtime
> > > > PM reference
> > > > URL   : https://patchwork.freedesktop.org/series/4135/
> > > > State : failure
> > > > 
> > > > == Summary ==
> > > > 
> > > > Series 4135v1 drm/i915: Wait until after wm optimization to
> > > > drop
> > > > runtime PM reference
> > > > http://patchwork.freedesktop.org/api/1.0/series/4135/revisions/
> > > > 1/mb
> > > > ox/
> > > > 
> > > > Test kms_flip:
> > > >         Subgroup basic-flip-vs-wf_vblank:
> > > >                 fail       -> PASS       (bsw-nuc-2)
> > > >         Subgroup basic-plain-flip:
> > > >                 dmesg-warn -> PASS       (hsw-gt2)
> > > > Test kms_force_connector_basic:
> > > >         Subgroup force-load-detect:
> > > >                 pass       -> SKIP       (ivb-t430s)
> > > 
> > > https://bugs.freedesktop.org/show_bug.cgi?id=93769
> > > 
> > > > Test kms_pipe_crc_basic:
> > > >         Subgroup read-crc-pipe-c:
> > > >                 dmesg-warn -> PASS       (hsw-gt2)
> > > >         Subgroup suspend-read-crc-pipe-a:
> > > >                 dmesg-warn -> PASS       (skl-i5k-2)
> > > >                 skip       -> PASS       (hsw-brixbox)
> > > >         Subgroup suspend-read-crc-pipe-c:
> > > >                 pass       -> DMESG-WARN (bsw-nuc-2)
> > > 
> > > https://bugs.freedesktop.org/show_bug.cgi?id=93294
> > > 
> > > >                 pass       -> INCOMPLETE (hsw-gt2)
> > > 
> > > Seems like the machine just died completely?  No
> > > stdout/stderr/command/dmesg output available from CI and time is
> > > given
> > > as 0:00:00.  Doesn't seem like it could be related to this patch.
> > > 
> > > 
> > > > Test pm_rpm:
> > > >         Subgroup basic-pci-d3-state:
> > > >                 dmesg-warn -> PASS       (snb-dellxps)
> > > >         Subgroup basic-rte:
> > > >                 pass       -> DMESG-WARN (bsw-nuc-2)
> > > 
> > > https://bugs.freedesktop.org/show_bug.cgi?id=94164
> > > 
> > > 
> > > Matt
> > 
> > Pushed to -dinq, thanks for the patch and the review.
> > 
> > I had to rebase it on top of Ville's recent
> > s/crtc_state/old_crtc_state/ change, please double check it.
> > 
> > Jani, this is for -fixes.
> 
> Surely you added either
> 
> Cc: drm-intel-fixes@lists.freedesktop.org
> 
> or
> 
> Cc: stable@vger.kernel.org
> 
> in the commit when you pushed, then?

No, I haven't will do that in the future. Btw, what's the rule for
deciding that something is for -fixes or stable only after it's been
pushed? Just ping you in case for -fixes and resend it in case of
stable?

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: ✗ Fi.CI.BAT: failure for drm/i915: Wait until after wm optimization to drop runtime PM reference
  2016-03-22 14:17         ` Imre Deak
@ 2016-03-22 15:56           ` Matt Roper
  2016-03-22 16:25             ` Imre Deak
  0 siblings, 1 reply; 16+ messages in thread
From: Matt Roper @ 2016-03-22 15:56 UTC (permalink / raw)
  To: Imre Deak; +Cc: Jani Nikula, intel-gfx

On Tue, Mar 22, 2016 at 04:17:58PM +0200, Imre Deak wrote:
> On ti, 2016-03-22 at 15:51 +0200, Jani Nikula wrote:
> > On Tue, 22 Mar 2016, Imre Deak <imre.deak@intel.com> wrote:
> > > [ text/plain ]
> > > On ti, 2016-03-08 at 09:26 -0800, Matt Roper wrote:
> > > > On Mon, Mar 07, 2016 at 11:47:51AM +0000, Patchwork wrote:
> > > > > == Series Details ==
> > > > > 
> > > > > Series: drm/i915: Wait until after wm optimization to drop
> > > > > runtime
> > > > > PM reference
> > > > > URL   : https://patchwork.freedesktop.org/series/4135/
> > > > > State : failure
> > > > > 
> > > > > == Summary ==
> > > > > 
> > > > > Series 4135v1 drm/i915: Wait until after wm optimization to
> > > > > drop
> > > > > runtime PM reference
> > > > > http://patchwork.freedesktop.org/api/1.0/series/4135/revisions/
> > > > > 1/mb
> > > > > ox/
> > > > > 
> > > > > Test kms_flip:
> > > > >         Subgroup basic-flip-vs-wf_vblank:
> > > > >                 fail       -> PASS       (bsw-nuc-2)
> > > > >         Subgroup basic-plain-flip:
> > > > >                 dmesg-warn -> PASS       (hsw-gt2)
> > > > > Test kms_force_connector_basic:
> > > > >         Subgroup force-load-detect:
> > > > >                 pass       -> SKIP       (ivb-t430s)
> > > > 
> > > > https://bugs.freedesktop.org/show_bug.cgi?id=93769
> > > > 
> > > > > Test kms_pipe_crc_basic:
> > > > >         Subgroup read-crc-pipe-c:
> > > > >                 dmesg-warn -> PASS       (hsw-gt2)
> > > > >         Subgroup suspend-read-crc-pipe-a:
> > > > >                 dmesg-warn -> PASS       (skl-i5k-2)
> > > > >                 skip       -> PASS       (hsw-brixbox)
> > > > >         Subgroup suspend-read-crc-pipe-c:
> > > > >                 pass       -> DMESG-WARN (bsw-nuc-2)
> > > > 
> > > > https://bugs.freedesktop.org/show_bug.cgi?id=93294
> > > > 
> > > > >                 pass       -> INCOMPLETE (hsw-gt2)
> > > > 
> > > > Seems like the machine just died completely?  No
> > > > stdout/stderr/command/dmesg output available from CI and time is
> > > > given
> > > > as 0:00:00.  Doesn't seem like it could be related to this patch.
> > > > 
> > > > 
> > > > > Test pm_rpm:
> > > > >         Subgroup basic-pci-d3-state:
> > > > >                 dmesg-warn -> PASS       (snb-dellxps)
> > > > >         Subgroup basic-rte:
> > > > >                 pass       -> DMESG-WARN (bsw-nuc-2)
> > > > 
> > > > https://bugs.freedesktop.org/show_bug.cgi?id=94164
> > > > 
> > > > 
> > > > Matt
> > > 
> > > Pushed to -dinq, thanks for the patch and the review.
> > > 
> > > I had to rebase it on top of Ville's recent
> > > s/crtc_state/old_crtc_state/ change, please double check it.
> > > 
> > > Jani, this is for -fixes.
> > 
> > Surely you added either
> > 
> > Cc: drm-intel-fixes@lists.freedesktop.org
> > 
> > or
> > 
> > Cc: stable@vger.kernel.org
> > 
> > in the commit when you pushed, then?
> 
> No, I haven't will do that in the future. Btw, what's the rule for
> deciding that something is for -fixes or stable only after it's been
> pushed? Just ping you in case for -fixes and resend it in case of
> stable?
> 
> --Imre

Are you sure we need this for -fixes?  The patch that introduced the
regression isn't in drm-next/4.6 as far as I know.


Matt

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: ✗ Fi.CI.BAT: failure for drm/i915: Wait until after wm optimization to drop runtime PM reference
  2016-03-22 15:56           ` Matt Roper
@ 2016-03-22 16:25             ` Imre Deak
  2016-03-22 16:31               ` Jani Nikula
  0 siblings, 1 reply; 16+ messages in thread
From: Imre Deak @ 2016-03-22 16:25 UTC (permalink / raw)
  To: Matt Roper; +Cc: Jani Nikula, intel-gfx

On ti, 2016-03-22 at 08:56 -0700, Matt Roper wrote:
> On Tue, Mar 22, 2016 at 04:17:58PM +0200, Imre Deak wrote:
> > On ti, 2016-03-22 at 15:51 +0200, Jani Nikula wrote:
> > > On Tue, 22 Mar 2016, Imre Deak <imre.deak@intel.com> wrote:
> > > > [ text/plain ]
> > > > On ti, 2016-03-08 at 09:26 -0800, Matt Roper wrote:
> > > > > On Mon, Mar 07, 2016 at 11:47:51AM +0000, Patchwork wrote:
> > > > > > == Series Details ==
> > > > > > 
> > > > > > Series: drm/i915: Wait until after wm optimization to drop
> > > > > > runtime
> > > > > > PM reference
> > > > > > URL   : https://patchwork.freedesktop.org/series/4135/
> > > > > > State : failure
> > > > > > 
> > > > > > == Summary ==
> > > > > > 
> > > > > > Series 4135v1 drm/i915: Wait until after wm optimization to
> > > > > > drop
> > > > > > runtime PM reference
> > > > > > http://patchwork.freedesktop.org/api/1.0/series/4135/revisi
> > > > > > ons/
> > > > > > 1/mb
> > > > > > ox/
> > > > > > 
> > > > > > Test kms_flip:
> > > > > >         Subgroup basic-flip-vs-wf_vblank:
> > > > > >                 fail       -> PASS       (bsw-nuc-2)
> > > > > >         Subgroup basic-plain-flip:
> > > > > >                 dmesg-warn -> PASS       (hsw-gt2)
> > > > > > Test kms_force_connector_basic:
> > > > > >         Subgroup force-load-detect:
> > > > > >                 pass       -> SKIP       (ivb-t430s)
> > > > > 
> > > > > https://bugs.freedesktop.org/show_bug.cgi?id=93769
> > > > > 
> > > > > > Test kms_pipe_crc_basic:
> > > > > >         Subgroup read-crc-pipe-c:
> > > > > >                 dmesg-warn -> PASS       (hsw-gt2)
> > > > > >         Subgroup suspend-read-crc-pipe-a:
> > > > > >                 dmesg-warn -> PASS       (skl-i5k-2)
> > > > > >                 skip       -> PASS       (hsw-brixbox)
> > > > > >         Subgroup suspend-read-crc-pipe-c:
> > > > > >                 pass       -> DMESG-WARN (bsw-nuc-2)
> > > > > 
> > > > > https://bugs.freedesktop.org/show_bug.cgi?id=93294
> > > > > 
> > > > > >                 pass       -> INCOMPLETE (hsw-gt2)
> > > > > 
> > > > > Seems like the machine just died completely?  No
> > > > > stdout/stderr/command/dmesg output available from CI and time
> > > > > is
> > > > > given
> > > > > as 0:00:00.  Doesn't seem like it could be related to this
> > > > > patch.
> > > > > 
> > > > > 
> > > > > > Test pm_rpm:
> > > > > >         Subgroup basic-pci-d3-state:
> > > > > >                 dmesg-warn -> PASS       (snb-dellxps)
> > > > > >         Subgroup basic-rte:
> > > > > >                 pass       -> DMESG-WARN (bsw-nuc-2)
> > > > > 
> > > > > https://bugs.freedesktop.org/show_bug.cgi?id=94164
> > > > > 
> > > > > 
> > > > > Matt
> > > > 
> > > > Pushed to -dinq, thanks for the patch and the review.
> > > > 
> > > > I had to rebase it on top of Ville's recent
> > > > s/crtc_state/old_crtc_state/ change, please double check it.
> > > > 
> > > > Jani, this is for -fixes.
> > > 
> > > Surely you added either
> > > 
> > > Cc: drm-intel-fixes@lists.freedesktop.org
> > > 
> > > or
> > > 
> > > Cc: stable@vger.kernel.org
> > > 
> > > in the commit when you pushed, then?
> > 
> > No, I haven't will do that in the future. Btw, what's the rule for
> > deciding that something is for -fixes or stable only after it's
> > been
> > pushed? Just ping you in case for -fixes and resend it in case of
> > stable?
> > 
> > --Imre
> 
> Are you sure we need this for -fixes?  The patch that introduced the
> regression isn't in drm-next/4.6 as far as I know.

The regressing commit ed4a6a7ca853 is in drm-intel-next, so I think we
do need it for -fixes.

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: ✗ Fi.CI.BAT: failure for drm/i915: Wait until after wm optimization to drop runtime PM reference
  2016-03-22 16:25             ` Imre Deak
@ 2016-03-22 16:31               ` Jani Nikula
  2016-03-23  8:38                 ` Daniel Vetter
  0 siblings, 1 reply; 16+ messages in thread
From: Jani Nikula @ 2016-03-22 16:31 UTC (permalink / raw)
  To: imre.deak, Matt Roper; +Cc: intel-gfx

On Tue, 22 Mar 2016, Imre Deak <imre.deak@intel.com> wrote:
> [ text/plain ]
> On ti, 2016-03-22 at 08:56 -0700, Matt Roper wrote:
>> On Tue, Mar 22, 2016 at 04:17:58PM +0200, Imre Deak wrote:
>> > On ti, 2016-03-22 at 15:51 +0200, Jani Nikula wrote:
>> > > On Tue, 22 Mar 2016, Imre Deak <imre.deak@intel.com> wrote:
>> > > > [ text/plain ]
>> > > > On ti, 2016-03-08 at 09:26 -0800, Matt Roper wrote:
>> > > > > On Mon, Mar 07, 2016 at 11:47:51AM +0000, Patchwork wrote:
>> > > > > > == Series Details ==
>> > > > > > 
>> > > > > > Series: drm/i915: Wait until after wm optimization to drop
>> > > > > > runtime
>> > > > > > PM reference
>> > > > > > URL   : https://patchwork.freedesktop.org/series/4135/
>> > > > > > State : failure
>> > > > > > 
>> > > > > > == Summary ==
>> > > > > > 
>> > > > > > Series 4135v1 drm/i915: Wait until after wm optimization to
>> > > > > > drop
>> > > > > > runtime PM reference
>> > > > > > http://patchwork.freedesktop.org/api/1.0/series/4135/revisi
>> > > > > > ons/
>> > > > > > 1/mb
>> > > > > > ox/
>> > > > > > 
>> > > > > > Test kms_flip:
>> > > > > >         Subgroup basic-flip-vs-wf_vblank:
>> > > > > >                 fail       -> PASS       (bsw-nuc-2)
>> > > > > >         Subgroup basic-plain-flip:
>> > > > > >                 dmesg-warn -> PASS       (hsw-gt2)
>> > > > > > Test kms_force_connector_basic:
>> > > > > >         Subgroup force-load-detect:
>> > > > > >                 pass       -> SKIP       (ivb-t430s)
>> > > > > 
>> > > > > https://bugs.freedesktop.org/show_bug.cgi?id=93769
>> > > > > 
>> > > > > > Test kms_pipe_crc_basic:
>> > > > > >         Subgroup read-crc-pipe-c:
>> > > > > >                 dmesg-warn -> PASS       (hsw-gt2)
>> > > > > >         Subgroup suspend-read-crc-pipe-a:
>> > > > > >                 dmesg-warn -> PASS       (skl-i5k-2)
>> > > > > >                 skip       -> PASS       (hsw-brixbox)
>> > > > > >         Subgroup suspend-read-crc-pipe-c:
>> > > > > >                 pass       -> DMESG-WARN (bsw-nuc-2)
>> > > > > 
>> > > > > https://bugs.freedesktop.org/show_bug.cgi?id=93294
>> > > > > 
>> > > > > >                 pass       -> INCOMPLETE (hsw-gt2)
>> > > > > 
>> > > > > Seems like the machine just died completely?  No
>> > > > > stdout/stderr/command/dmesg output available from CI and time
>> > > > > is
>> > > > > given
>> > > > > as 0:00:00.  Doesn't seem like it could be related to this
>> > > > > patch.
>> > > > > 
>> > > > > 
>> > > > > > Test pm_rpm:
>> > > > > >         Subgroup basic-pci-d3-state:
>> > > > > >                 dmesg-warn -> PASS       (snb-dellxps)
>> > > > > >         Subgroup basic-rte:
>> > > > > >                 pass       -> DMESG-WARN (bsw-nuc-2)
>> > > > > 
>> > > > > https://bugs.freedesktop.org/show_bug.cgi?id=94164
>> > > > > 
>> > > > > 
>> > > > > Matt
>> > > > 
>> > > > Pushed to -dinq, thanks for the patch and the review.
>> > > > 
>> > > > I had to rebase it on top of Ville's recent
>> > > > s/crtc_state/old_crtc_state/ change, please double check it.
>> > > > 
>> > > > Jani, this is for -fixes.
>> > > 
>> > > Surely you added either
>> > > 
>> > > Cc: drm-intel-fixes@lists.freedesktop.org
>> > > 
>> > > or
>> > > 
>> > > Cc: stable@vger.kernel.org
>> > > 
>> > > in the commit when you pushed, then?
>> > 
>> > No, I haven't will do that in the future. Btw, what's the rule for
>> > deciding that something is for -fixes or stable only after it's
>> > been
>> > pushed? Just ping you in case for -fixes and resend it in case of
>> > stable?
>> > 
>> > --Imre
>> 
>> Are you sure we need this for -fixes?  The patch that introduced the
>> regression isn't in drm-next/4.6 as far as I know.
>
> The regressing commit ed4a6a7ca853 is in drm-intel-next, so I think we
> do need it for -fixes.

Ah. It's not in 4.5 and it's not on its way to 4.6. Not fixes material.

BR,
Jani.



-- 
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] 16+ messages in thread

* Re: ✗ Fi.CI.BAT: failure for drm/i915: Wait until after wm optimization to drop runtime PM reference
  2016-03-22 16:31               ` Jani Nikula
@ 2016-03-23  8:38                 ` Daniel Vetter
  0 siblings, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2016-03-23  8:38 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

On Tue, Mar 22, 2016 at 06:31:53PM +0200, Jani Nikula wrote:
> On Tue, 22 Mar 2016, Imre Deak <imre.deak@intel.com> wrote:
> > [ text/plain ]
> > On ti, 2016-03-22 at 08:56 -0700, Matt Roper wrote:
> >> On Tue, Mar 22, 2016 at 04:17:58PM +0200, Imre Deak wrote:
> >> > On ti, 2016-03-22 at 15:51 +0200, Jani Nikula wrote:
> >> > > On Tue, 22 Mar 2016, Imre Deak <imre.deak@intel.com> wrote:
> >> > > > [ text/plain ]
> >> > > > On ti, 2016-03-08 at 09:26 -0800, Matt Roper wrote:
> >> > > > > On Mon, Mar 07, 2016 at 11:47:51AM +0000, Patchwork wrote:
> >> > > > > > == Series Details ==
> >> > > > > > 
> >> > > > > > Series: drm/i915: Wait until after wm optimization to drop
> >> > > > > > runtime
> >> > > > > > PM reference
> >> > > > > > URL   : https://patchwork.freedesktop.org/series/4135/
> >> > > > > > State : failure
> >> > > > > > 
> >> > > > > > == Summary ==
> >> > > > > > 
> >> > > > > > Series 4135v1 drm/i915: Wait until after wm optimization to
> >> > > > > > drop
> >> > > > > > runtime PM reference
> >> > > > > > http://patchwork.freedesktop.org/api/1.0/series/4135/revisi
> >> > > > > > ons/
> >> > > > > > 1/mb
> >> > > > > > ox/
> >> > > > > > 
> >> > > > > > Test kms_flip:
> >> > > > > >         Subgroup basic-flip-vs-wf_vblank:
> >> > > > > >                 fail       -> PASS       (bsw-nuc-2)
> >> > > > > >         Subgroup basic-plain-flip:
> >> > > > > >                 dmesg-warn -> PASS       (hsw-gt2)
> >> > > > > > Test kms_force_connector_basic:
> >> > > > > >         Subgroup force-load-detect:
> >> > > > > >                 pass       -> SKIP       (ivb-t430s)
> >> > > > > 
> >> > > > > https://bugs.freedesktop.org/show_bug.cgi?id=93769
> >> > > > > 
> >> > > > > > Test kms_pipe_crc_basic:
> >> > > > > >         Subgroup read-crc-pipe-c:
> >> > > > > >                 dmesg-warn -> PASS       (hsw-gt2)
> >> > > > > >         Subgroup suspend-read-crc-pipe-a:
> >> > > > > >                 dmesg-warn -> PASS       (skl-i5k-2)
> >> > > > > >                 skip       -> PASS       (hsw-brixbox)
> >> > > > > >         Subgroup suspend-read-crc-pipe-c:
> >> > > > > >                 pass       -> DMESG-WARN (bsw-nuc-2)
> >> > > > > 
> >> > > > > https://bugs.freedesktop.org/show_bug.cgi?id=93294
> >> > > > > 
> >> > > > > >                 pass       -> INCOMPLETE (hsw-gt2)
> >> > > > > 
> >> > > > > Seems like the machine just died completely?  No
> >> > > > > stdout/stderr/command/dmesg output available from CI and time
> >> > > > > is
> >> > > > > given
> >> > > > > as 0:00:00.  Doesn't seem like it could be related to this
> >> > > > > patch.
> >> > > > > 
> >> > > > > 
> >> > > > > > Test pm_rpm:
> >> > > > > >         Subgroup basic-pci-d3-state:
> >> > > > > >                 dmesg-warn -> PASS       (snb-dellxps)
> >> > > > > >         Subgroup basic-rte:
> >> > > > > >                 pass       -> DMESG-WARN (bsw-nuc-2)
> >> > > > > 
> >> > > > > https://bugs.freedesktop.org/show_bug.cgi?id=94164
> >> > > > > 
> >> > > > > 
> >> > > > > Matt
> >> > > > 
> >> > > > Pushed to -dinq, thanks for the patch and the review.
> >> > > > 
> >> > > > I had to rebase it on top of Ville's recent
> >> > > > s/crtc_state/old_crtc_state/ change, please double check it.
> >> > > > 
> >> > > > Jani, this is for -fixes.
> >> > > 
> >> > > Surely you added either
> >> > > 
> >> > > Cc: drm-intel-fixes@lists.freedesktop.org
> >> > > 
> >> > > or
> >> > > 
> >> > > Cc: stable@vger.kernel.org
> >> > > 
> >> > > in the commit when you pushed, then?
> >> > 
> >> > No, I haven't will do that in the future. Btw, what's the rule for
> >> > deciding that something is for -fixes or stable only after it's
> >> > been
> >> > pushed? Just ping you in case for -fixes and resend it in case of
> >> > stable?
> >> > 
> >> > --Imre
> >> 
> >> Are you sure we need this for -fixes?  The patch that introduced the
> >> regression isn't in drm-next/4.6 as far as I know.
> >
> > The regressing commit ed4a6a7ca853 is in drm-intel-next, so I think we
> > do need it for -fixes.
> 
> Ah. It's not in 4.5 and it's not on its way to 4.6. Not fixes material.

For the future:

$ dim tag-contains $commit_that_youre_fixing

Will tell you either the kernel version of if it hasn't landed, the
branches. And if it's in airlied/drm-next but dinq is already past the
merge window then it needs Cc: stable. If it's only in dinq then not.

I guess we could improve the heuristics of the script a bit and directly
print out the Cc: stable or Cc: drm-intel-fixes line you would need, if
any ... Volunteers highly welcome ;-)

-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2016-03-23  8:37 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-04 23:59 [PATCH] drm/i915: Wait until after wm optimization to drop runtime PM reference Matt Roper
2016-03-05  1:25 ` Imre Deak
2016-03-07 16:10   ` Matt Roper
2016-03-07 16:28     ` Imre Deak
2016-03-07 11:47 ` ✗ Fi.CI.BAT: failure for " Patchwork
2016-03-08 17:26   ` Matt Roper
2016-03-22 12:55     ` Imre Deak
2016-03-22 13:51       ` Jani Nikula
2016-03-22 14:17         ` Imre Deak
2016-03-22 15:56           ` Matt Roper
2016-03-22 16:25             ` Imre Deak
2016-03-22 16:31               ` Jani Nikula
2016-03-23  8:38                 ` Daniel Vetter
2016-03-07 11:53 ` [PATCH] " Maarten Lankhorst
2016-03-07 16:26   ` Matt Roper
2016-03-22 11:04 ` Ville Syrjälä

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.