[PATCH] drm/i915: Grab power domain in skl_pipe_wm_get_hw_state()

[PATCH] drm/i915: Grab power domain in skl_pipe_wm_get_hw_state()

[Maarten Lankhorst]

This should get rid of unclaimed register debug warnings, if
it still happens we should put this in a intel_crtc->active check..

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104172
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index ab6f1b770891..52d157c00535 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5477,6 +5477,11 @@ void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc,
 	int level, max_level;
 	enum plane_id plane_id;
 	uint32_t val;
+	enum intel_display_power_domain power_domain;
+
+	power_domain = POWER_DOMAIN_PIPE(pipe);
+	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
+		return;
 
 	max_level = ilk_wm_max_level(dev_priv);
 
@@ -5500,10 +5505,9 @@ void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc,
 		skl_wm_level_from_reg_val(val, &wm->trans_wm);
 	}
 
-	if (!intel_crtc->active)
-		return;
-
 	out->linetime = I915_READ(PIPE_WM_LINETIME(pipe));
+
+	intel_display_power_put(dev_priv, power_domain);
 }
 
 void skl_wm_get_hw_state(struct drm_device *dev)
-- 
2.15.1

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

✓ Fi.CI.BAT: success for drm/i915: Grab power domain in skl_pipe_wm_get_hw_state()

[Patchwork]

== Series Details ==

Series: drm/i915: Grab power domain in skl_pipe_wm_get_hw_state()
URL   : https://patchwork.freedesktop.org/series/35577/
State : success

== Summary ==

Series 35577v1 drm/i915: Grab power domain in skl_pipe_wm_get_hw_state()
https://patchwork.freedesktop.org/api/1.0/series/35577/revisions/1/mbox/

Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                dmesg-warn -> PASS       (fi-kbl-r) fdo#104172

fdo#104172 https://bugs.freedesktop.org/show_bug.cgi?id=104172

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:434s
fi-bdw-gvtdvm    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:438s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:383s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:490s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:275s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:500s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:493s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:479s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:462s
fi-elk-e7500     total:224  pass:163  dwarn:15  dfail:0   fail:0   skip:45 
fi-gdg-551       total:288  pass:178  dwarn:1   dfail:0   fail:1   skip:108 time:261s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:531s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:414s
fi-hsw-4770r     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:417s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:389s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:479s
fi-ivb-3770      total:288  pass:255  dwarn:0   dfail:0   fail:0   skip:33  time:421s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:487s
fi-kbl-7560u     total:288  pass:268  dwarn:1   dfail:0   fail:0   skip:19  time:514s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:471s
fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:526s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:578s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:445s
fi-skl-6600u     total:288  pass:260  dwarn:1   dfail:0   fail:0   skip:27  time:529s
fi-skl-6700hq    total:288  pass:261  dwarn:1   dfail:0   fail:0   skip:26  time:557s
fi-skl-6700k2    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:508s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:481s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:445s
fi-snb-2520m     total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:547s
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:417s
Blacklisted hosts:
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:588s
fi-cnl-y         total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:627s

1a9d5c66e6def1c7a8b3334abf70f940edc90adb drm-tip: 2017y-12m-19d-14h-25m-30s UTC integration manifest
ea5de102287a drm/i915: Grab power domain in skl_pipe_wm_get_hw_state()

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7538/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for drm/i915: Grab power domain in skl_pipe_wm_get_hw_state()

[Patchwork]

== Series Details ==

Series: drm/i915: Grab power domain in skl_pipe_wm_get_hw_state()
URL   : https://patchwork.freedesktop.org/series/35577/
State : success

== Summary ==

Test kms_frontbuffer_tracking:
        Subgroup fbc-1p-offscren-pri-shrfb-draw-blt:
                pass       -> FAIL       (shard-snb) fdo#101623 +1
Test gem_eio:
        Subgroup in-flight-contexts:
                dmesg-warn -> PASS       (shard-snb) fdo#104058
Test kms_flip:
        Subgroup wf_vblank-vs-modeset-interruptible:
                pass       -> DMESG-WARN (shard-hsw) fdo#102614
Test gem_tiled_swapping:
        Subgroup non-threaded:
                pass       -> INCOMPLETE (shard-hsw) fdo#104218
Test drv_suspend:
        Subgroup fence-restore-tiled2untiled:
                dmesg-warn -> PASS       (shard-snb)

fdo#101623 https://bugs.freedesktop.org/show_bug.cgi?id=101623
fdo#104058 https://bugs.freedesktop.org/show_bug.cgi?id=104058
fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
fdo#104218 https://bugs.freedesktop.org/show_bug.cgi?id=104218

shard-hsw        total:2683 pass:1516 dwarn:2   dfail:0   fail:10  skip:1154 time:9233s
shard-snb        total:2712 pass:1308 dwarn:1   dfail:0   fail:12  skip:1391 time:8091s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7538/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Grab power domain in skl_pipe_wm_get_hw_state()

[Rodrigo Vivi]

On Tue, Dec 19, 2017 at 12:16:45PM +0000, Maarten Lankhorst wrote:
> This should get rid of unclaimed register debug warnings, if
> it still happens we should put this in a intel_crtc->active check..

What if we just call skl_pipe_wm_get_hw_state() on skl_wm_get_hw_state()
if intel_crtc->active ?

-   skl_pipe_wm_get_hw_state(crtc, &cstate->wm.skl.optimal);
-                if (intel_crtc->active)
-                        hw->dirty_pipes |= drm_crtc_mask(crtc);

+                if (intel_crtc->active) {
+                        hw->dirty_pipes |= drm_crtc_mask(crtc);
+			 skl_pipe_wm_get_hw_state(crtc, &cstate->wm.skl.optimal);
+		 }

>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104172
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index ab6f1b770891..52d157c00535 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5477,6 +5477,11 @@ void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc,
>  	int level, max_level;
>  	enum plane_id plane_id;
>  	uint32_t val;
> +	enum intel_display_power_domain power_domain;
> +
> +	power_domain = POWER_DOMAIN_PIPE(pipe);
> +	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
> +		return;
>
>  	max_level = ilk_wm_max_level(dev_priv);
>
> @@ -5500,10 +5505,9 @@ void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc,
>  		skl_wm_level_from_reg_val(val, &wm->trans_wm);
>  	}
>
> -	if (!intel_crtc->active)
> -		return;

but with my way or this way I wonder if we are now changing some expected wm
behavior since on the current version we just skip late if crtc wasn't active.

> -
>  	out->linetime = I915_READ(PIPE_WM_LINETIME(pipe));
> +
> +	intel_display_power_put(dev_priv, power_domain);
>  }
>
>  void skl_wm_get_hw_state(struct drm_device *dev)
> --
> 2.15.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Grab power domain in skl_pipe_wm_get_hw_state()

[Pandiyan, Dhinakaran]

On Tue, 2017-12-19 at 13:16 +0100, Maarten Lankhorst wrote:
> This should get rid of unclaimed register debug warnings, if
> it still happens we should put this in a intel_crtc->active check..
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104172


The bugzilla indicates this is a regression from 
"drm/i915: Restore GT performance in headless mode with DMC loaded",
which seems a bit odd to me. That patch should not have disabled a power
well which was enabled before. If anything, it should fix unclaimed
register accesses. 



> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index ab6f1b770891..52d157c00535 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5477,6 +5477,11 @@ void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc,
>  	int level, max_level;
>  	enum plane_id plane_id;
>  	uint32_t val;
> +	enum intel_display_power_domain power_domain;
> +
> +	power_domain = POWER_DOMAIN_PIPE(pipe);
> +	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
> +		return;
>  
>  	max_level = ilk_wm_max_level(dev_priv);
>  
> @@ -5500,10 +5505,9 @@ void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc,
>  		skl_wm_level_from_reg_val(val, &wm->trans_wm);
>  	}
>  
> -	if (!intel_crtc->active)
> -		return;
> -

Doing a power_domain_get_if_enabled() before reading the register seems
like the right thing to do, but is power_get_if_enabled() expected to be
correct at this point? The reason I ask is,
modeset_get_crtc_power_domains() is called after skl_wm_get_hw_state().
So, doesn't that mean the pipe power domain might not have been acquired
even if the CRTC is active.



>  	out->linetime = I915_READ(PIPE_WM_LINETIME(pipe));
> +
> +	intel_display_power_put(dev_priv, power_domain);
>  }
>  
>  void skl_wm_get_hw_state(struct drm_device *dev)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Grab power domain in skl_pipe_wm_get_hw_state()

[Pandiyan, Dhinakaran]

On Tue, 2018-01-02 at 18:33 +0000, Pandiyan, Dhinakaran wrote:
> On Tue, 2017-12-19 at 13:16 +0100, Maarten Lankhorst wrote:
> > This should get rid of unclaimed register debug warnings, if
> > it still happens we should put this in a intel_crtc->active check..
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104172
> 
> 
> The bugzilla indicates this is a regression from 
> "drm/i915: Restore GT performance in headless mode with DMC loaded",
> which seems a bit odd to me. That patch should not have disabled a power
> well which was enabled before. If anything, it should fix unclaimed
> register accesses. 
> 
The comments in the bug report don't really confirm that the unclaimed
register access is a regression from "drm/i915: Restore GT performance
in headless mode with DMC loaded".

> 
> 
> > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_pm.c | 10 +++++++---
> >  1 file changed, 7 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index ab6f1b770891..52d157c00535 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -5477,6 +5477,11 @@ void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc,
> >  	int level, max_level;
> >  	enum plane_id plane_id;
> >  	uint32_t val;
> > +	enum intel_display_power_domain power_domain;
> > +
> > +	power_domain = POWER_DOMAIN_PIPE(pipe);
> > +	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
> > +		return;
> >  
> >  	max_level = ilk_wm_max_level(dev_priv);
> >  
> > @@ -5500,10 +5505,9 @@ void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc,
> >  		skl_wm_level_from_reg_val(val, &wm->trans_wm);
> >  	}
> >  
> > -	if (!intel_crtc->active)
> > -		return;
> > -
> 
> Doing a power_domain_get_if_enabled() before reading the register seems
> like the right thing to do, but is power_get_if_enabled() expected to be
> correct at this point? The reason I ask is,
> modeset_get_crtc_power_domains() is called after skl_wm_get_hw_state().
> So, doesn't that mean the pipe power domain might not have been acquired
> even if the CRTC is active.
To answer my own question, POWER_DOMAIN_INIT should have taken care of
this. So, this looks good to me.

> 
> 
> 
> >  	out->linetime = I915_READ(PIPE_WM_LINETIME(pipe));

This looks okay too, but can you confirm updating this field for
inactive CRTCs is not a problem? With that, 
Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>

> > +
> > +	intel_display_power_put(dev_priv, power_domain);
> >  }
> >  
> >  void skl_wm_get_hw_state(struct drm_device *dev)
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Grab power domain in skl_pipe_wm_get_hw_state()

[Imre Deak]

Hi,

On Tue, Dec 19, 2017 at 01:16:45PM +0100, Maarten Lankhorst wrote:
> This should get rid of unclaimed register debug warnings, if
> it still happens we should put this in a intel_crtc->active check..
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104172
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index ab6f1b770891..52d157c00535 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5477,6 +5477,11 @@ void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc,
>  	int level, max_level;
>  	enum plane_id plane_id;
>  	uint32_t val;
> +	enum intel_display_power_domain power_domain;
> +
> +	power_domain = POWER_DOMAIN_PIPE(pipe);
> +	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
> +		return;

just wondering how the lack of this could lead to the above bug. We have
intel_display_set_init_power(dev_priv, true); early during resume and
then intel_display_set_init_power(dev_priv, false); at the end of
intel_modeset_setup_hw_state(). That should've kept pipe B's power well
(PW#2) on.

There's the following in the above log during resume, just before the
unclaimed access:

<7>[  509.317871] [drm:intel_dump_pipe_config [i915]] [CRTC:37:pipe A][setup_hw_state]
<7>[  509.317889] [drm:intel_dump_pipe_config [i915]] output_types:  (0x0)
<7>[  509.317909] [drm:intel_power_well_disable [i915]] disabling power well 2
<7>[  509.317925] [drm:intel_dump_pipe_config [i915]] cpu_transcoder: A, pipe bpp: 0, dithering: 0

So PW#2 gets disabled asynchronously, although AFAICS no async display
stuff should run before intel_modeset_setup_hw_state() returns. Maybe
something not canceled properly during suspend()?

--Imre

>  
>  	max_level = ilk_wm_max_level(dev_priv);
>  
> @@ -5500,10 +5505,9 @@ void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc,
>  		skl_wm_level_from_reg_val(val, &wm->trans_wm);
>  	}
>  
> -	if (!intel_crtc->active)
> -		return;
> -
>  	out->linetime = I915_READ(PIPE_WM_LINETIME(pipe));
> +
> +	intel_display_power_put(dev_priv, power_domain);
>  }
>  
>  void skl_wm_get_hw_state(struct drm_device *dev)
> -- 
> 2.15.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx