All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] drm/i915/psr: Fix race in intel_psr_work()
@ 2018-06-25  5:47 Dhinakaran Pandiyan
  2018-06-25  5:47 ` [PATCH 2/2] drm/i915/psr: Warn for erroneous enabling of both PSR1 and PSR2 Dhinakaran Pandiyan
                   ` (5 more replies)
  0 siblings, 6 replies; 12+ messages in thread
From: Dhinakaran Pandiyan @ 2018-06-25  5:47 UTC (permalink / raw)
  To: intel-gfx; +Cc: Dhinakaran Pandiyan, Rodrigo Vivi

Commit 5422b37c907e ("drm/i915/psr: Kill delays when activating psr
back.") switched from delayed work to the plain variant and while doing so
removed the check for work_busy() before scheduling a PSR activation.
This appears to cause consecutive executions of psr_activate() in this
scenario - after a worker picks up the PSR work item for execution and
before the work function can acquire the PSR mutex, a psr_flush() can
get hold of the mutex and schedule another PSR work. Without a psr_exit()
between the two psr_activate() calls, warning messages get printed.
Further, since we drop the mutex in the midst of psr_work() to wait for
PSR to idle, another work item can also get scheduled. Fix this by
returning if PSR was already active.

Fixes: 5422b37c907e ("drm/i915/psr: Kill delays when activating psr back.")
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106948
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: José Roberto de Souza <jose.souza@intel.com>
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_psr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index aea81ace854b..7aa324f0d1f7 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -811,7 +811,7 @@ static void intel_psr_work(struct work_struct *work)
 	 * recheck. Since psr_flush first clears this and then reschedules we
 	 * won't ever miss a flush when bailing out here.
 	 */
-	if (dev_priv->psr.busy_frontbuffer_bits)
+	if (dev_priv->psr.busy_frontbuffer_bits || dev_priv->psr.active)
 		goto unlock;
 
 	intel_psr_activate(dev_priv->psr.enabled);
-- 
2.14.1

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

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

* [PATCH 2/2] drm/i915/psr: Warn for erroneous enabling of both PSR1 and PSR2.
  2018-06-25  5:47 [PATCH 1/2] drm/i915/psr: Fix race in intel_psr_work() Dhinakaran Pandiyan
@ 2018-06-25  5:47 ` Dhinakaran Pandiyan
  2018-06-26  0:09   ` Rodrigo Vivi
  2018-06-26  9:05   ` [PATCH v2 " Dhinakaran Pandiyan
  2018-06-25  6:21 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915/psr: Fix race in intel_psr_work() Patchwork
                   ` (4 subsequent siblings)
  5 siblings, 2 replies; 12+ messages in thread
From: Dhinakaran Pandiyan @ 2018-06-25  5:47 UTC (permalink / raw)
  To: intel-gfx; +Cc: Dhinakaran Pandiyan

Depending whether PSR1 or PSR2 was configured, we print a warning if the
corresponding control mmio indicated PSR was erroneously enabled. As
Chris pointed out, it makes more sense to check for both the mmio's
since we expect neither PSR1 nor PSR2 to be enabled when psr_activate() is
called.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 drivers/gpu/drm/i915/intel_psr.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 7aa324f0d1f7..970b8ced46a3 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -576,10 +576,8 @@ static void intel_psr_activate(struct intel_dp *intel_dp)
 	struct drm_device *dev = intel_dig_port->base.base.dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
 
-	if (dev_priv->psr.psr2_enabled)
-		WARN_ON(I915_READ(EDP_PSR2_CTL) & EDP_PSR2_ENABLE);
-	else
-		WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE);
+	WARN_ON(I915_READ(EDP_PSR2_CTL) & EDP_PSR2_ENABLE);
+	WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE);
 	WARN_ON(dev_priv->psr.active);
 	lockdep_assert_held(&dev_priv->psr.lock);
 
-- 
2.14.1

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

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

* ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915/psr: Fix race in intel_psr_work()
  2018-06-25  5:47 [PATCH 1/2] drm/i915/psr: Fix race in intel_psr_work() Dhinakaran Pandiyan
  2018-06-25  5:47 ` [PATCH 2/2] drm/i915/psr: Warn for erroneous enabling of both PSR1 and PSR2 Dhinakaran Pandiyan
@ 2018-06-25  6:21 ` Patchwork
  2018-06-25  7:46 ` ✓ Fi.CI.IGT: " Patchwork
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2018-06-25  6:21 UTC (permalink / raw)
  To: Dhinakaran Pandiyan; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/2] drm/i915/psr: Fix race in intel_psr_work()
URL   : https://patchwork.freedesktop.org/series/45316/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4371 -> Patchwork_9407 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/45316/revisions/1/mbox/

== Known issues ==

  Here are the changes found in Patchwork_9407 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@debugfs_test@read_all_entries:
      fi-snb-2520m:       PASS -> INCOMPLETE (fdo#103713)

    igt@gem_exec_gttfill@basic:
      fi-byt-n2820:       PASS -> FAIL (fdo#106744)

    igt@gem_exec_suspend@basic-s4-devices:
      fi-kbl-7500u:       PASS -> DMESG-WARN (fdo#105128)

    
  fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
  fdo#105128 https://bugs.freedesktop.org/show_bug.cgi?id=105128
  fdo#106744 https://bugs.freedesktop.org/show_bug.cgi?id=106744


== Participating hosts (40 -> 35) ==

  Missing    (5): fi-kbl-x1275 fi-ilk-m540 fi-glk-dsi fi-bsw-cyan fi-hsw-4200u 


== Build changes ==

    * Linux: CI_DRM_4371 -> Patchwork_9407

  CI_DRM_4371: 9094e9d97a6e13db8c1a444d08c0988adad9a002 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4528: 6be300d405de5974b262e8b93a445be4ac618e6a @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9407: 6c2fb59839b58fb0634484eec26adfe9eadc937b @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

6c2fb59839b5 drm/i915/psr: Warn for erroneous enabling of both PSR1 and PSR2.
f48e0bd04c8f drm/i915/psr: Fix race in intel_psr_work()

== Logs ==

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

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

* ✓ Fi.CI.IGT: success for series starting with [1/2] drm/i915/psr: Fix race in intel_psr_work()
  2018-06-25  5:47 [PATCH 1/2] drm/i915/psr: Fix race in intel_psr_work() Dhinakaran Pandiyan
  2018-06-25  5:47 ` [PATCH 2/2] drm/i915/psr: Warn for erroneous enabling of both PSR1 and PSR2 Dhinakaran Pandiyan
  2018-06-25  6:21 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915/psr: Fix race in intel_psr_work() Patchwork
@ 2018-06-25  7:46 ` Patchwork
  2018-06-26  0:10 ` [PATCH 1/2] " Rodrigo Vivi
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2018-06-25  7:46 UTC (permalink / raw)
  To: Dhinakaran Pandiyan; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/2] drm/i915/psr: Fix race in intel_psr_work()
URL   : https://patchwork.freedesktop.org/series/45316/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4371_full -> Patchwork_9407_full =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_9407_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_9407_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_9407_full:

  === IGT changes ===

    ==== Warnings ====

    igt@gem_mocs_settings@mocs-rc6-render:
      shard-kbl:          PASS -> SKIP

    igt@kms_cursor_legacy@flip-vs-cursor-busy-crc-legacy:
      shard-snb:          SKIP -> PASS +1

    igt@pm_rc6_residency@rc6-accuracy:
      shard-kbl:          SKIP -> PASS

    
== Known issues ==

  Here are the changes found in Patchwork_9407_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_selftest@live_gtt:
      shard-glk:          PASS -> FAIL (fdo#105347)

    igt@drv_suspend@shrink:
      shard-apl:          PASS -> INCOMPLETE (fdo#103927)

    igt@gem_exec_await@wide-contexts:
      shard-kbl:          PASS -> FAIL (fdo#105900)

    igt@kms_flip@plain-flip-fb-recreate-interruptible:
      shard-glk:          PASS -> FAIL (fdo#100368)

    igt@kms_flip_tiling@flip-y-tiled:
      shard-glk:          PASS -> FAIL (fdo#104724, fdo#103822)

    igt@kms_setmode@basic:
      shard-snb:          PASS -> FAIL (fdo#99912)

    
    ==== Possible fixes ====

    igt@kms_cursor_legacy@2x-nonblocking-modeset-vs-cursor-atomic:
      shard-glk:          FAIL (fdo#106509, fdo#105454) -> PASS

    igt@kms_flip@flip-vs-expired-vblank-interruptible:
      shard-glk:          FAIL (fdo#105363) -> PASS

    igt@kms_flip_tiling@flip-to-x-tiled:
      shard-glk:          FAIL (fdo#104724, fdo#103822) -> PASS

    
  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#103822 https://bugs.freedesktop.org/show_bug.cgi?id=103822
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
  fdo#104724 https://bugs.freedesktop.org/show_bug.cgi?id=104724
  fdo#105347 https://bugs.freedesktop.org/show_bug.cgi?id=105347
  fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
  fdo#105454 https://bugs.freedesktop.org/show_bug.cgi?id=105454
  fdo#105900 https://bugs.freedesktop.org/show_bug.cgi?id=105900
  fdo#106509 https://bugs.freedesktop.org/show_bug.cgi?id=106509
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


== Participating hosts (5 -> 5) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4371 -> Patchwork_9407

  CI_DRM_4371: 9094e9d97a6e13db8c1a444d08c0988adad9a002 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4528: 6be300d405de5974b262e8b93a445be4ac618e6a @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9407: 6c2fb59839b58fb0634484eec26adfe9eadc937b @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

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

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

* Re: [PATCH 2/2] drm/i915/psr: Warn for erroneous enabling of both PSR1 and PSR2.
  2018-06-25  5:47 ` [PATCH 2/2] drm/i915/psr: Warn for erroneous enabling of both PSR1 and PSR2 Dhinakaran Pandiyan
@ 2018-06-26  0:09   ` Rodrigo Vivi
  2018-06-26  6:44     ` Dhinakaran Pandiyan
  2018-06-26  9:05   ` [PATCH v2 " Dhinakaran Pandiyan
  1 sibling, 1 reply; 12+ messages in thread
From: Rodrigo Vivi @ 2018-06-26  0:09 UTC (permalink / raw)
  To: Dhinakaran Pandiyan; +Cc: intel-gfx, Dhinakaran Pandiyan

On Sun, Jun 24, 2018 at 10:47:41PM -0700, Dhinakaran Pandiyan wrote:
> Depending whether PSR1 or PSR2 was configured, we print a warning if the
> corresponding control mmio indicated PSR was erroneously enabled. As
> Chris pointed out, it makes more sense to check for both the mmio's
> since we expect neither PSR1 nor PSR2 to be enabled when psr_activate() is
> called.
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_psr.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index 7aa324f0d1f7..970b8ced46a3 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -576,10 +576,8 @@ static void intel_psr_activate(struct intel_dp *intel_dp)
>  	struct drm_device *dev = intel_dig_port->base.base.dev;
>  	struct drm_i915_private *dev_priv = to_i915(dev);
>  
> -	if (dev_priv->psr.psr2_enabled)
> -		WARN_ON(I915_READ(EDP_PSR2_CTL) & EDP_PSR2_ENABLE);
> -	else
> -		WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE);
> +	WARN_ON(I915_READ(EDP_PSR2_CTL) & EDP_PSR2_ENABLE);

now you have to check for platform which really has this register
to avoid unclaimed registers accesses...

> +	WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE);
>  	WARN_ON(dev_priv->psr.active);
>  	lockdep_assert_held(&dev_priv->psr.lock);
>  
> -- 
> 2.14.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

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

* Re: [PATCH 1/2] drm/i915/psr: Fix race in intel_psr_work()
  2018-06-25  5:47 [PATCH 1/2] drm/i915/psr: Fix race in intel_psr_work() Dhinakaran Pandiyan
                   ` (2 preceding siblings ...)
  2018-06-25  7:46 ` ✓ Fi.CI.IGT: " Patchwork
@ 2018-06-26  0:10 ` Rodrigo Vivi
  2018-06-26  9:42 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915/psr: Fix race in intel_psr_work() (rev2) Patchwork
  2018-06-26 11:26 ` ✓ Fi.CI.IGT: " Patchwork
  5 siblings, 0 replies; 12+ messages in thread
From: Rodrigo Vivi @ 2018-06-26  0:10 UTC (permalink / raw)
  To: Dhinakaran Pandiyan; +Cc: intel-gfx, Dhinakaran Pandiyan

On Sun, Jun 24, 2018 at 10:47:40PM -0700, Dhinakaran Pandiyan wrote:
> Commit 5422b37c907e ("drm/i915/psr: Kill delays when activating psr
> back.") switched from delayed work to the plain variant and while doing so
> removed the check for work_busy() before scheduling a PSR activation.
> This appears to cause consecutive executions of psr_activate() in this
> scenario - after a worker picks up the PSR work item for execution and
> before the work function can acquire the PSR mutex, a psr_flush() can
> get hold of the mutex and schedule another PSR work. Without a psr_exit()
> between the two psr_activate() calls, warning messages get printed.
> Further, since we drop the mutex in the midst of psr_work() to wait for
> PSR to idle, another work item can also get scheduled. Fix this by
> returning if PSR was already active.

:( my bad again, sorry...

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> 
> Fixes: 5422b37c907e ("drm/i915/psr: Kill delays when activating psr back.")
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106948
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: José Roberto de Souza <jose.souza@intel.com>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/intel_psr.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index aea81ace854b..7aa324f0d1f7 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -811,7 +811,7 @@ static void intel_psr_work(struct work_struct *work)
>  	 * recheck. Since psr_flush first clears this and then reschedules we
>  	 * won't ever miss a flush when bailing out here.
>  	 */
> -	if (dev_priv->psr.busy_frontbuffer_bits)
> +	if (dev_priv->psr.busy_frontbuffer_bits || dev_priv->psr.active)
>  		goto unlock;
>  
>  	intel_psr_activate(dev_priv->psr.enabled);
> -- 
> 2.14.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

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

* Re: [PATCH 2/2] drm/i915/psr: Warn for erroneous enabling of both PSR1 and PSR2.
  2018-06-26  0:09   ` Rodrigo Vivi
@ 2018-06-26  6:44     ` Dhinakaran Pandiyan
  0 siblings, 0 replies; 12+ messages in thread
From: Dhinakaran Pandiyan @ 2018-06-26  6:44 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx

On Mon, 2018-06-25 at 17:09 -0700, Rodrigo Vivi wrote:
> On Sun, Jun 24, 2018 at 10:47:41PM -0700, Dhinakaran Pandiyan wrote:
> > 
> > Depending whether PSR1 or PSR2 was configured, we print a warning
> > if the
> > corresponding control mmio indicated PSR was erroneously enabled.
> > As
> > Chris pointed out, it makes more sense to check for both the mmio's
> > since we expect neither PSR1 nor PSR2 to be enabled when
> > psr_activate() is
> > called.
> > 
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_psr.c | 6 ++----
> >  1 file changed, 2 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > b/drivers/gpu/drm/i915/intel_psr.c
> > index 7aa324f0d1f7..970b8ced46a3 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -576,10 +576,8 @@ static void intel_psr_activate(struct intel_dp
> > *intel_dp)
> >  	struct drm_device *dev = intel_dig_port->base.base.dev;
> >  	struct drm_i915_private *dev_priv = to_i915(dev);
> >  
> > -	if (dev_priv->psr.psr2_enabled)
> > -		WARN_ON(I915_READ(EDP_PSR2_CTL) &
> > EDP_PSR2_ENABLE);
> > -	else
> > -		WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE);
> > +	WARN_ON(I915_READ(EDP_PSR2_CTL) & EDP_PSR2_ENABLE);
> now you have to check for platform which really has this register
> to avoid unclaimed registers accesses...
> 
I'll add this?
	if (INTEL_GEN(dev_priv) > 9)
		WARN_ON(I915_READ(EDP_PSR2_CTL) & EDP_PSR2_ENABLE);


> > 
> > +	WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE);
> >  	WARN_ON(dev_priv->psr.active);
> >  	lockdep_assert_held(&dev_priv->psr.lock);
> >  
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* [PATCH v2 2/2] drm/i915/psr: Warn for erroneous enabling of both PSR1 and PSR2.
  2018-06-25  5:47 ` [PATCH 2/2] drm/i915/psr: Warn for erroneous enabling of both PSR1 and PSR2 Dhinakaran Pandiyan
  2018-06-26  0:09   ` Rodrigo Vivi
@ 2018-06-26  9:05   ` Dhinakaran Pandiyan
  2018-06-26 15:50     ` Rodrigo Vivi
  1 sibling, 1 reply; 12+ messages in thread
From: Dhinakaran Pandiyan @ 2018-06-26  9:05 UTC (permalink / raw)
  To: intel-gfx; +Cc: Dhinakaran Pandiyan, Rodrigo Vivi

Depending whether PSR1 or PSR2 was configured, we print a warning if the
corresponding control mmio indicated PSR was erroneously enabled. As
Chris pointed out, it makes more sense to check for both the mmio's
since we expect neither PSR1 nor PSR2 to be enabled when psr_activate() is
called.

v2: Read PSR2 control register only on supported platforms (Rodrigo)

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 drivers/gpu/drm/i915/intel_psr.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 7aa324f0d1f7..f27193310480 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -576,10 +576,9 @@ static void intel_psr_activate(struct intel_dp *intel_dp)
 	struct drm_device *dev = intel_dig_port->base.base.dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
 
-	if (dev_priv->psr.psr2_enabled)
+	if (INTEL_GEN(dev_priv) >= 9)
 		WARN_ON(I915_READ(EDP_PSR2_CTL) & EDP_PSR2_ENABLE);
-	else
-		WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE);
+	WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE);
 	WARN_ON(dev_priv->psr.active);
 	lockdep_assert_held(&dev_priv->psr.lock);
 
-- 
2.14.1

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

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

* ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915/psr: Fix race in intel_psr_work() (rev2)
  2018-06-25  5:47 [PATCH 1/2] drm/i915/psr: Fix race in intel_psr_work() Dhinakaran Pandiyan
                   ` (3 preceding siblings ...)
  2018-06-26  0:10 ` [PATCH 1/2] " Rodrigo Vivi
@ 2018-06-26  9:42 ` Patchwork
  2018-06-26 11:26 ` ✓ Fi.CI.IGT: " Patchwork
  5 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2018-06-26  9:42 UTC (permalink / raw)
  To: Dhinakaran Pandiyan; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/2] drm/i915/psr: Fix race in intel_psr_work() (rev2)
URL   : https://patchwork.freedesktop.org/series/45316/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4377 -> Patchwork_9422 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/45316/revisions/2/mbox/

== Known issues ==

  Here are the changes found in Patchwork_9422 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_frontbuffer_tracking@basic:
      fi-hsw-peppy:       PASS -> DMESG-FAIL (fdo#102614, fdo#106103)

    
    ==== Possible fixes ====

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-snb-2520m:       INCOMPLETE (fdo#103713) -> PASS

    
  fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
  fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
  fdo#106103 https://bugs.freedesktop.org/show_bug.cgi?id=106103


== Participating hosts (44 -> 39) ==

  Missing    (5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u 


== Build changes ==

    * Linux: CI_DRM_4377 -> Patchwork_9422

  CI_DRM_4377: 6482eeda80b98506834716a1655ec35a774ec95f @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4530: 0e98bf69f146eb72fe3a7c3b19a049b5786f0ca3 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9422: 939622df0848e5a917ea314a9b80eb54b8940d7c @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

939622df0848 drm/i915/psr: Warn for erroneous enabling of both PSR1 and PSR2.
33c57b2c40d9 drm/i915/psr: Fix race in intel_psr_work()

== Logs ==

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

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

* ✓ Fi.CI.IGT: success for series starting with [1/2] drm/i915/psr: Fix race in intel_psr_work() (rev2)
  2018-06-25  5:47 [PATCH 1/2] drm/i915/psr: Fix race in intel_psr_work() Dhinakaran Pandiyan
                   ` (4 preceding siblings ...)
  2018-06-26  9:42 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915/psr: Fix race in intel_psr_work() (rev2) Patchwork
@ 2018-06-26 11:26 ` Patchwork
  5 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2018-06-26 11:26 UTC (permalink / raw)
  To: Dhinakaran Pandiyan; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/2] drm/i915/psr: Fix race in intel_psr_work() (rev2)
URL   : https://patchwork.freedesktop.org/series/45316/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4377_full -> Patchwork_9422_full =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_9422_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_9422_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_9422_full:

  === IGT changes ===

    ==== Warnings ====

    igt@gem_exec_schedule@deep-bsd2:
      shard-kbl:          SKIP -> PASS +2

    igt@gem_exec_schedule@deep-vebox:
      shard-kbl:          PASS -> SKIP +1

    
== Known issues ==

  Here are the changes found in Patchwork_9422_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_ctx_isolation@rcs0-s3:
      shard-kbl:          PASS -> INCOMPLETE (fdo#103665)

    igt@gem_ctx_switch@basic-all-light:
      shard-hsw:          PASS -> INCOMPLETE (fdo#103540)

    igt@kms_atomic_transition@1x-modeset-transitions-nonblocking-fencing:
      shard-glk:          PASS -> FAIL (fdo#105703)

    igt@kms_flip@flip-vs-expired-vblank-interruptible:
      shard-glk:          PASS -> FAIL (fdo#105363, fdo#102887)

    igt@kms_flip@modeset-vs-vblank-race:
      shard-glk:          PASS -> FAIL (fdo#103060)

    igt@kms_flip_tiling@flip-to-x-tiled:
      shard-glk:          PASS -> FAIL (fdo#104724)

    igt@kms_setmode@basic:
      shard-kbl:          PASS -> FAIL (fdo#99912)

    
    ==== Possible fixes ====

    igt@drv_selftest@live_hangcheck:
      shard-apl:          DMESG-FAIL (fdo#106560, fdo#106947) -> PASS

    igt@kms_atomic_transition@1x-modeset-transitions-nonblocking:
      shard-glk:          FAIL (fdo#105703) -> PASS

    igt@kms_cursor_legacy@2x-nonblocking-modeset-vs-cursor-atomic:
      shard-glk:          FAIL (fdo#105454, fdo#106509) -> PASS

    igt@kms_flip_tiling@flip-y-tiled:
      shard-glk:          FAIL (fdo#104724, fdo#103822) -> PASS

    igt@kms_setmode@basic:
      shard-hsw:          FAIL (fdo#99912) -> PASS

    
  fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
  fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
  fdo#103540 https://bugs.freedesktop.org/show_bug.cgi?id=103540
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#103822 https://bugs.freedesktop.org/show_bug.cgi?id=103822
  fdo#104724 https://bugs.freedesktop.org/show_bug.cgi?id=104724
  fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
  fdo#105454 https://bugs.freedesktop.org/show_bug.cgi?id=105454
  fdo#105703 https://bugs.freedesktop.org/show_bug.cgi?id=105703
  fdo#106509 https://bugs.freedesktop.org/show_bug.cgi?id=106509
  fdo#106560 https://bugs.freedesktop.org/show_bug.cgi?id=106560
  fdo#106947 https://bugs.freedesktop.org/show_bug.cgi?id=106947
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


== Participating hosts (5 -> 5) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4377 -> Patchwork_9422

  CI_DRM_4377: 6482eeda80b98506834716a1655ec35a774ec95f @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4530: 0e98bf69f146eb72fe3a7c3b19a049b5786f0ca3 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9422: 939622df0848e5a917ea314a9b80eb54b8940d7c @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

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

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

* Re: [PATCH v2 2/2] drm/i915/psr: Warn for erroneous enabling of both PSR1 and PSR2.
  2018-06-26  9:05   ` [PATCH v2 " Dhinakaran Pandiyan
@ 2018-06-26 15:50     ` Rodrigo Vivi
  2018-06-26 20:02       ` Dhinakaran Pandiyan
  0 siblings, 1 reply; 12+ messages in thread
From: Rodrigo Vivi @ 2018-06-26 15:50 UTC (permalink / raw)
  To: Dhinakaran Pandiyan; +Cc: intel-gfx, Dhinakaran Pandiyan

On Tue, Jun 26, 2018 at 02:05:22AM -0700, Dhinakaran Pandiyan wrote:
> Depending whether PSR1 or PSR2 was configured, we print a warning if the
> corresponding control mmio indicated PSR was erroneously enabled. As
> Chris pointed out, it makes more sense to check for both the mmio's
> since we expect neither PSR1 nor PSR2 to be enabled when psr_activate() is
> called.
> 
> v2: Read PSR2 control register only on supported platforms (Rodrigo)
> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_psr.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index 7aa324f0d1f7..f27193310480 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -576,10 +576,9 @@ static void intel_psr_activate(struct intel_dp *intel_dp)
>  	struct drm_device *dev = intel_dig_port->base.base.dev;
>  	struct drm_i915_private *dev_priv = to_i915(dev);
>  
> -	if (dev_priv->psr.psr2_enabled)
> +	if (INTEL_GEN(dev_priv) >= 9)
>  		WARN_ON(I915_READ(EDP_PSR2_CTL) & EDP_PSR2_ENABLE);
> -	else
> -		WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE);
> +	WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE);
>  	WARN_ON(dev_priv->psr.active);
>  	lockdep_assert_held(&dev_priv->psr.lock);
>  
> -- 
> 2.14.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

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

* Re: [PATCH v2 2/2] drm/i915/psr: Warn for erroneous enabling of both PSR1 and PSR2.
  2018-06-26 15:50     ` Rodrigo Vivi
@ 2018-06-26 20:02       ` Dhinakaran Pandiyan
  0 siblings, 0 replies; 12+ messages in thread
From: Dhinakaran Pandiyan @ 2018-06-26 20:02 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx

On Tue, 2018-06-26 at 08:50 -0700, Rodrigo Vivi wrote:
> On Tue, Jun 26, 2018 at 02:05:22AM -0700, Dhinakaran Pandiyan wrote:
> > 
> > Depending whether PSR1 or PSR2 was configured, we print a warning
> > if the
> > corresponding control mmio indicated PSR was erroneously enabled.
> > As
> > Chris pointed out, it makes more sense to check for both the mmio's
> > since we expect neither PSR1 nor PSR2 to be enabled when
> > psr_activate() is
> > called.
> > 
> > v2: Read PSR2 control register only on supported platforms
> > (Rodrigo)
> > 
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

I have pushed this series, thanks for the reviews.

-DK
> 
> > 
> > ---
> >  drivers/gpu/drm/i915/intel_psr.c | 5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > b/drivers/gpu/drm/i915/intel_psr.c
> > index 7aa324f0d1f7..f27193310480 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -576,10 +576,9 @@ static void intel_psr_activate(struct intel_dp
> > *intel_dp)
> >  	struct drm_device *dev = intel_dig_port->base.base.dev;
> >  	struct drm_i915_private *dev_priv = to_i915(dev);
> >  
> > -	if (dev_priv->psr.psr2_enabled)
> > +	if (INTEL_GEN(dev_priv) >= 9)
> >  		WARN_ON(I915_READ(EDP_PSR2_CTL) &
> > EDP_PSR2_ENABLE);
> > -	else
> > -		WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE);
> > +	WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE);
> >  	WARN_ON(dev_priv->psr.active);
> >  	lockdep_assert_held(&dev_priv->psr.lock);
> >  
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

end of thread, other threads:[~2018-06-26 19:36 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-25  5:47 [PATCH 1/2] drm/i915/psr: Fix race in intel_psr_work() Dhinakaran Pandiyan
2018-06-25  5:47 ` [PATCH 2/2] drm/i915/psr: Warn for erroneous enabling of both PSR1 and PSR2 Dhinakaran Pandiyan
2018-06-26  0:09   ` Rodrigo Vivi
2018-06-26  6:44     ` Dhinakaran Pandiyan
2018-06-26  9:05   ` [PATCH v2 " Dhinakaran Pandiyan
2018-06-26 15:50     ` Rodrigo Vivi
2018-06-26 20:02       ` Dhinakaran Pandiyan
2018-06-25  6:21 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915/psr: Fix race in intel_psr_work() Patchwork
2018-06-25  7:46 ` ✓ Fi.CI.IGT: " Patchwork
2018-06-26  0:10 ` [PATCH 1/2] " Rodrigo Vivi
2018-06-26  9:42 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915/psr: Fix race in intel_psr_work() (rev2) Patchwork
2018-06-26 11:26 ` ✓ Fi.CI.IGT: " Patchwork

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