All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915/skl: Ensure HW is powered during DDB HW state readout
@ 2016-02-17 14:31 Imre Deak
  2016-02-17 14:55 ` Jani Nikula
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Imre Deak @ 2016-02-17 14:31 UTC (permalink / raw)
  To: intel-gfx; +Cc: Mika Kuoppala

The assumption when adding the intel_display_power_is_enabled() checks
was that if it returns success the power can't be turned off afterwards
during the HW access, which is guaranteed by modeset locks. This isn't
always true, so make sure we hold a dedicated reference for the time of
the access.

Spotted-by: Mika Kuoppala <mika.kuoppala@intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93441
CC: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index b63cdb2..347d4df 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2851,7 +2851,10 @@ void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv,
 	memset(ddb, 0, sizeof(*ddb));
 
 	for_each_pipe(dev_priv, pipe) {
-		if (!intel_display_power_is_enabled(dev_priv, POWER_DOMAIN_PIPE(pipe)))
+		enum intel_display_power_domain power_domain;
+
+		power_domain = POWER_DOMAIN_PIPE(pipe);
+		if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
 			continue;
 
 		for_each_plane(dev_priv, pipe, plane) {
@@ -2863,6 +2866,8 @@ void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv,
 		val = I915_READ(CUR_BUF_CFG(pipe));
 		skl_ddb_entry_init_from_hw(&ddb->plane[pipe][PLANE_CURSOR],
 					   val);
+
+		intel_display_power_put(dev_priv, power_domain);
 	}
 }
 
-- 
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/skl: Ensure HW is powered during DDB HW state readout
  2016-02-17 14:31 [PATCH] drm/i915/skl: Ensure HW is powered during DDB HW state readout Imre Deak
@ 2016-02-17 14:55 ` Jani Nikula
  2016-02-17 16:40   ` Imre Deak
  2016-02-17 15:30 ` Mika Kuoppala
  2016-02-17 15:47 ` ✗ Fi.CI.BAT: failure for " Patchwork
  2 siblings, 1 reply; 7+ messages in thread
From: Jani Nikula @ 2016-02-17 14:55 UTC (permalink / raw)
  To: Imre Deak, intel-gfx; +Cc: Mika Kuoppala

On Wed, 17 Feb 2016, Imre Deak <imre.deak@intel.com> wrote:
> The assumption when adding the intel_display_power_is_enabled() checks
> was that if it returns success the power can't be turned off afterwards
> during the HW access, which is guaranteed by modeset locks. This isn't
> always true, so make sure we hold a dedicated reference for the time of
> the access.
>
> Spotted-by: Mika Kuoppala <mika.kuoppala@intel.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93441
> CC: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Imre Deak <imre.deak@intel.com>

Cc: fixes or stable? Fixes which commit?

BR,
Jani.

> ---
>  drivers/gpu/drm/i915/intel_pm.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index b63cdb2..347d4df 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2851,7 +2851,10 @@ void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv,
>  	memset(ddb, 0, sizeof(*ddb));
>  
>  	for_each_pipe(dev_priv, pipe) {
> -		if (!intel_display_power_is_enabled(dev_priv, POWER_DOMAIN_PIPE(pipe)))
> +		enum intel_display_power_domain power_domain;
> +
> +		power_domain = POWER_DOMAIN_PIPE(pipe);
> +		if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
>  			continue;
>  
>  		for_each_plane(dev_priv, pipe, plane) {
> @@ -2863,6 +2866,8 @@ void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv,
>  		val = I915_READ(CUR_BUF_CFG(pipe));
>  		skl_ddb_entry_init_from_hw(&ddb->plane[pipe][PLANE_CURSOR],
>  					   val);
> +
> +		intel_display_power_put(dev_priv, power_domain);
>  	}
>  }

-- 
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/skl: Ensure HW is powered during DDB HW state readout
  2016-02-17 14:31 [PATCH] drm/i915/skl: Ensure HW is powered during DDB HW state readout Imre Deak
  2016-02-17 14:55 ` Jani Nikula
@ 2016-02-17 15:30 ` Mika Kuoppala
  2016-02-17 15:47 ` ✗ Fi.CI.BAT: failure for " Patchwork
  2 siblings, 0 replies; 7+ messages in thread
From: Mika Kuoppala @ 2016-02-17 15:30 UTC (permalink / raw)
  To: Imre Deak, intel-gfx

Imre Deak <imre.deak@intel.com> writes:

> The assumption when adding the intel_display_power_is_enabled() checks
> was that if it returns success the power can't be turned off afterwards
> during the HW access, which is guaranteed by modeset locks. This isn't
> always true, so make sure we hold a dedicated reference for the time of
> the access.
>
> Spotted-by: Mika Kuoppala <mika.kuoppala@intel.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93441
> CC: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Imre Deak <imre.deak@intel.com>

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_pm.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index b63cdb2..347d4df 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2851,7 +2851,10 @@ void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv,
>  	memset(ddb, 0, sizeof(*ddb));
>  
>  	for_each_pipe(dev_priv, pipe) {
> -		if (!intel_display_power_is_enabled(dev_priv, POWER_DOMAIN_PIPE(pipe)))
> +		enum intel_display_power_domain power_domain;
> +
> +		power_domain = POWER_DOMAIN_PIPE(pipe);
> +		if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
>  			continue;
>  
>  		for_each_plane(dev_priv, pipe, plane) {
> @@ -2863,6 +2866,8 @@ void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv,
>  		val = I915_READ(CUR_BUF_CFG(pipe));
>  		skl_ddb_entry_init_from_hw(&ddb->plane[pipe][PLANE_CURSOR],
>  					   val);
> +
> +		intel_display_power_put(dev_priv, power_domain);
>  	}
>  }
>  
> -- 
> 2.5.0
_______________________________________________
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: failure for drm/i915/skl: Ensure HW is powered during DDB HW state readout
  2016-02-17 14:31 [PATCH] drm/i915/skl: Ensure HW is powered during DDB HW state readout Imre Deak
  2016-02-17 14:55 ` Jani Nikula
  2016-02-17 15:30 ` Mika Kuoppala
@ 2016-02-17 15:47 ` Patchwork
  2016-02-17 16:45   ` Imre Deak
  2 siblings, 1 reply; 7+ messages in thread
From: Patchwork @ 2016-02-17 15:47 UTC (permalink / raw)
  To: Imre Deak; +Cc: intel-gfx

== Summary ==

Series 3532v1 drm/i915/skl: Ensure HW is powered during DDB HW state readout
http://patchwork.freedesktop.org/api/1.0/series/3532/revisions/1/mbox/

Test drv_hangman:
        Subgroup error-state-basic:
                incomplete -> PASS       (snb-dellxps)
Test kms_flip:
        Subgroup basic-flip-vs-dpms:
                dmesg-warn -> PASS       (ilk-hp8440p) UNSTABLE
        Subgroup basic-flip-vs-modeset:
                pass       -> DMESG-WARN (ilk-hp8440p) UNSTABLE
Test kms_force_connector_basic:
        Subgroup force-load-detect:
                skip       -> FAIL       (ilk-hp8440p)
Test pm_rpm:
        Subgroup basic-rte:
                pass       -> DMESG-WARN (bsw-nuc-2)

bdw-nuci7        total:162  pass:152  dwarn:0   dfail:0   fail:0   skip:10 
bdw-ultra        total:165  pass:152  dwarn:0   dfail:0   fail:0   skip:13 
bsw-nuc-2        total:165  pass:134  dwarn:1   dfail:0   fail:1   skip:29 
byt-nuc          total:165  pass:141  dwarn:0   dfail:0   fail:0   skip:24 
hsw-gt2          total:165  pass:154  dwarn:0   dfail:0   fail:1   skip:10 
ilk-hp8440p      total:165  pass:115  dwarn:1   dfail:0   fail:1   skip:48 
ivb-t430s        total:165  pass:150  dwarn:0   dfail:0   fail:1   skip:14 
skl-i5k-2        total:165  pass:149  dwarn:1   dfail:0   fail:0   skip:15 
snb-dellxps      total:165  pass:142  dwarn:0   dfail:0   fail:1   skip:22 
snb-x220t        total:165  pass:142  dwarn:0   dfail:0   fail:2   skip:21 

Results at /archive/results/CI_IGT_test/Patchwork_1426/

a868b844abfbce5f574665319f51d397416ae49b drm-intel-nightly: 2016y-02m-17d-14h-18m-14s UTC integration manifest
c13931c633bfe7f522ffdd1c3b63378847863d49 drm/i915/skl: Ensure HW is powered during DDB HW state readout

_______________________________________________
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/skl: Ensure HW is powered during DDB HW state readout
  2016-02-17 14:55 ` Jani Nikula
@ 2016-02-17 16:40   ` Imre Deak
  0 siblings, 0 replies; 7+ messages in thread
From: Imre Deak @ 2016-02-17 16:40 UTC (permalink / raw)
  To: Jani Nikula, intel-gfx; +Cc: Mika Kuoppala

On ke, 2016-02-17 at 16:55 +0200, Jani Nikula wrote:
> On Wed, 17 Feb 2016, Imre Deak <imre.deak@intel.com> wrote:
> > The assumption when adding the intel_display_power_is_enabled()
> > checks
> > was that if it returns success the power can't be turned off
> > afterwards
> > during the HW access, which is guaranteed by modeset locks. This
> > isn't
> > always true, so make sure we hold a dedicated reference for the
> > time of
> > the access.
> > 
> > Spotted-by: Mika Kuoppala <mika.kuoppala@intel.com>
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93441
> > CC: Chris Wilson <chris@chris-wilson.co.uk>
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> 
> Cc: fixes or stable? Fixes which commit?

Hm, yes forgot about this. We need it in -fixes since it may fix DMC
functionality according to Mika. Imo no need for it in stable, since on
other platforms it wouldn't fix anything besides HW state checking.

I pushed the "drm/i915: add missing display power refs" patchset and
this patch rebased on -fixes to
https://github.com/ideak/linux/commits/display_power_get_if_enabled-for-fixes

--Imre

> BR,
> Jani.
> 
> > ---
> >  drivers/gpu/drm/i915/intel_pm.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c
> > b/drivers/gpu/drm/i915/intel_pm.c
> > index b63cdb2..347d4df 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -2851,7 +2851,10 @@ void skl_ddb_get_hw_state(struct
> > drm_i915_private *dev_priv,
> >  	memset(ddb, 0, sizeof(*ddb));
> >  
> >  	for_each_pipe(dev_priv, pipe) {
> > -		if (!intel_display_power_is_enabled(dev_priv,
> > POWER_DOMAIN_PIPE(pipe)))
> > +		enum intel_display_power_domain power_domain;
> > +
> > +		power_domain = POWER_DOMAIN_PIPE(pipe);
> > +		if (!intel_display_power_get_if_enabled(dev_priv,
> > power_domain))
> >  			continue;
> >  
> >  		for_each_plane(dev_priv, pipe, plane) {
> > @@ -2863,6 +2866,8 @@ void skl_ddb_get_hw_state(struct
> > drm_i915_private *dev_priv,
> >  		val = I915_READ(CUR_BUF_CFG(pipe));
> >  		skl_ddb_entry_init_from_hw(&ddb-
> > >plane[pipe][PLANE_CURSOR],
> >  					   val);
> > +
> > +		intel_display_power_put(dev_priv, power_domain);
> >  	}
> >  }
> 
_______________________________________________
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: failure for drm/i915/skl: Ensure HW is powered during DDB HW state readout
  2016-02-17 15:47 ` ✗ Fi.CI.BAT: failure for " Patchwork
@ 2016-02-17 16:45   ` Imre Deak
  2016-02-18 14:05     ` Imre Deak
  0 siblings, 1 reply; 7+ messages in thread
From: Imre Deak @ 2016-02-17 16:45 UTC (permalink / raw)
  To: intel-gfx

On ke, 2016-02-17 at 15:47 +0000, Patchwork wrote:
> == Summary ==
> 
> Series 3532v1 drm/i915/skl: Ensure HW is powered during DDB HW state
> readout
> http://patchwork.freedesktop.org/api/1.0/series/3532/revisions/1/mbox
> /
> 
> Test drv_hangman:
>         Subgroup error-state-basic:
>                 incomplete -> PASS       (snb-dellxps)
> Test kms_flip:
>         Subgroup basic-flip-vs-dpms:
>                 dmesg-warn -> PASS       (ilk-hp8440p) UNSTABLE
>         Subgroup basic-flip-vs-modeset:
>                 pass       -> DMESG-WARN (ilk-hp8440p) UNSTABLE

Clearly unrelated, since the patch is SKL specific.

> Test kms_force_connector_basic:
>         Subgroup force-load-detect:
>                 skip       -> FAIL       (ilk-hp8440p)

The same as above.

> Test pm_rpm:
>         Subgroup basic-rte:
>                 pass       -> DMESG-WARN (bsw-nuc-2)

Ditto.

> 
> bdw-
> nuci7        total:162  pass:152  dwarn:0   dfail:0   fail:0   skip:1
> 0 
> bdw-
> ultra        total:165  pass:152  dwarn:0   dfail:0   fail:0   skip:1
> 3 
> bsw-nuc-
> 2        total:165  pass:134  dwarn:1   dfail:0   fail:1   skip:29 
> byt-
> nuc          total:165  pass:141  dwarn:0   dfail:0   fail:0   skip:2
> 4 
> hsw-
> gt2          total:165  pass:154  dwarn:0   dfail:0   fail:1   skip:1
> 0 
> ilk-
> hp8440p      total:165  pass:115  dwarn:1   dfail:0   fail:1   skip:4
> 8 
> ivb-
> t430s        total:165  pass:150  dwarn:0   dfail:0   fail:1   skip:1
> 4 
> skl-i5k-
> 2        total:165  pass:149  dwarn:1   dfail:0   fail:0   skip:15 
> snb-
> dellxps      total:165  pass:142  dwarn:0   dfail:0   fail:1   skip:2
> 2 
> snb-
> x220t        total:165  pass:142  dwarn:0   dfail:0   fail:2   skip:2
> 1 
> 
> Results at /archive/results/CI_IGT_test/Patchwork_1426/
> 
> a868b844abfbce5f574665319f51d397416ae49b drm-intel-nightly: 2016y-
> 02m-17d-14h-18m-14s UTC integration manifest
> c13931c633bfe7f522ffdd1c3b63378847863d49 drm/i915/skl: Ensure HW is
> powered during DDB HW state readout
> 
_______________________________________________
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: failure for drm/i915/skl: Ensure HW is powered during DDB HW state readout
  2016-02-17 16:45   ` Imre Deak
@ 2016-02-18 14:05     ` Imre Deak
  0 siblings, 0 replies; 7+ messages in thread
From: Imre Deak @ 2016-02-18 14:05 UTC (permalink / raw)
  To: intel-gfx; +Cc: Jani Nikula

On ke, 2016-02-17 at 18:45 +0200, Imre Deak wrote:
> On ke, 2016-02-17 at 15:47 +0000, Patchwork wrote:
> > == Summary ==
> > 
> > Series 3532v1 drm/i915/skl: Ensure HW is powered during DDB HW
> > state
> > readout
> > http://patchwork.freedesktop.org/api/1.0/series/3532/revisions/1/mb
> > ox
> > /
> > 
> > Test drv_hangman:
> >         Subgroup error-state-basic:
> >                 incomplete -> PASS       (snb-dellxps)
> > Test kms_flip:
> >         Subgroup basic-flip-vs-dpms:
> >                 dmesg-warn -> PASS       (ilk-hp8440p) UNSTABLE
> >         Subgroup basic-flip-vs-modeset:
> >                 pass       -> DMESG-WARN (ilk-hp8440p) UNSTABLE
> 
> Clearly unrelated, since the patch is SKL specific.
> 
> > Test kms_force_connector_basic:
> >         Subgroup force-load-detect:
> >                 skip       -> FAIL       (ilk-hp8440p)
> 
> The same as above.
> 
> > Test pm_rpm:
> >         Subgroup basic-rte:
> >                 pass       -> DMESG-WARN (bsw-nuc-2)
> 
> Ditto.

Pushed to -dinq, thanks for the review.

> 
> > 
> > bdw-
> > nuci7        total:162  pass:152  dwarn:0   dfail:0   fail:0   skip
> > :1
> > 0 
> > bdw-
> > ultra        total:165  pass:152  dwarn:0   dfail:0   fail:0   skip
> > :1
> > 3 
> > bsw-nuc-
> > 2        total:165  pass:134  dwarn:1   dfail:0   fail:1   skip:29 
> > byt-
> > nuc          total:165  pass:141  dwarn:0   dfail:0   fail:0   skip
> > :2
> > 4 
> > hsw-
> > gt2          total:165  pass:154  dwarn:0   dfail:0   fail:1   skip
> > :1
> > 0 
> > ilk-
> > hp8440p      total:165  pass:115  dwarn:1   dfail:0   fail:1   skip
> > :4
> > 8 
> > ivb-
> > t430s        total:165  pass:150  dwarn:0   dfail:0   fail:1   skip
> > :1
> > 4 
> > skl-i5k-
> > 2        total:165  pass:149  dwarn:1   dfail:0   fail:0   skip:15 
> > snb-
> > dellxps      total:165  pass:142  dwarn:0   dfail:0   fail:1   skip
> > :2
> > 2 
> > snb-
> > x220t        total:165  pass:142  dwarn:0   dfail:0   fail:2   skip
> > :2
> > 1 
> > 
> > Results at /archive/results/CI_IGT_test/Patchwork_1426/
> > 
> > a868b844abfbce5f574665319f51d397416ae49b drm-intel-nightly: 2016y-
> > 02m-17d-14h-18m-14s UTC integration manifest
> > c13931c633bfe7f522ffdd1c3b63378847863d49 drm/i915/skl: Ensure HW is
> > powered during DDB HW state readout
> > 
> _______________________________________________
> 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

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

end of thread, other threads:[~2016-02-18 14:05 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-17 14:31 [PATCH] drm/i915/skl: Ensure HW is powered during DDB HW state readout Imre Deak
2016-02-17 14:55 ` Jani Nikula
2016-02-17 16:40   ` Imre Deak
2016-02-17 15:30 ` Mika Kuoppala
2016-02-17 15:47 ` ✗ Fi.CI.BAT: failure for " Patchwork
2016-02-17 16:45   ` Imre Deak
2016-02-18 14:05     ` Imre Deak

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.