Intel-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] drm/i915/psr: Lockless version of psr_wait_for_idle
@ 2018-06-22  7:05 Tarun Vyas
  2018-06-22  7:12 ` Chris Wilson
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Tarun Vyas @ 2018-06-22  7:05 UTC (permalink / raw)
  To: intel-gfx; +Cc: dhinakaran.pandiyan, rodrigo.vivi

This is a lockless version of the exisiting psr_wait_for_idle().
We want to wait for PSR to idle out inside intel_pipe_update_start.
At the time of a pipe update, we should never race with any psr
enable or disable code, which is a part of crtc enable/disable. So,
we can live w/o taking any psr locks at all.
The follow up patch will use this lockless wait inside pipe_update_
start to wait for PSR to idle out before checking for vblank evasion.

Even if psr is never enabled, psr2_enabled will be false and this
function will wait for PSR1 to idle out, which should just return
immediately, so a very short (~1-2 usec) wait for cases where PSR
is disabled.

v2: Add comment to explain the 25msec timeout (DK)

Signed-off-by: Tarun Vyas <tarun.vyas@intel.com>
---
 drivers/gpu/drm/i915/intel_drv.h |  1 +
 drivers/gpu/drm/i915/intel_psr.c | 24 ++++++++++++++++++++++++
 2 files changed, 25 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 578346b8d7e2..a48aad0f99bf 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1920,6 +1920,7 @@ void intel_psr_compute_config(struct intel_dp *intel_dp,
 			      struct intel_crtc_state *crtc_state);
 void intel_psr_irq_control(struct drm_i915_private *dev_priv, bool debug);
 void intel_psr_irq_handler(struct drm_i915_private *dev_priv, u32 psr_iir);
+void psr_wait_for_idle_lockless(struct drm_i915_private *dev_priv);
 
 /* intel_runtime_pm.c */
 int intel_power_domains_init(struct drm_i915_private *);
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index aea81ace854b..8e69e6193063 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -757,6 +757,30 @@ void intel_psr_disable(struct intel_dp *intel_dp,
 	cancel_work_sync(&dev_priv->psr.work);
 }
 
+void psr_wait_for_idle_lockless(struct drm_i915_private *dev_priv)
+{
+	i915_reg_t reg;
+	u32 mask;
+	int err;
+
+	if (dev_priv->psr.psr2_enabled) {
+		reg = EDP_PSR2_STATUS;
+		mask = EDP_PSR2_STATUS_STATE_MASK;
+	} else {
+		reg = EDP_PSR_STATUS;
+		mask = EDP_PSR_STATUS_STATE_MASK;
+	}
+
+	/*
+	 * The  25 msec timeout accounts for a frame @ 60Hz refresh rate,
+	 * exit training an aux handshake time.
+	 */
+	err = intel_wait_for_register(dev_priv, reg, mask,
+				      EDP_PSR_STATUS_STATE_IDLE, 25);
+	if (err)
+		DRM_ERROR("Timed out waiting for PSR Idle for pipe update\n");
+}
+
 static bool psr_wait_for_idle(struct drm_i915_private *dev_priv)
 {
 	struct intel_dp *intel_dp;
-- 
2.13.5

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

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

* Re: [PATCH v2] drm/i915/psr: Lockless version of psr_wait_for_idle
  2018-06-22  7:05 [PATCH v2] drm/i915/psr: Lockless version of psr_wait_for_idle Tarun Vyas
@ 2018-06-22  7:12 ` Chris Wilson
  2018-06-22  7:26   ` Tarun Vyas
  2018-06-22  7:39 ` ✓ Fi.CI.BAT: success for drm/i915/psr: Lockless version of psr_wait_for_idle (rev3) Patchwork
  2018-06-22 14:24 ` ✓ Fi.CI.IGT: " Patchwork
  2 siblings, 1 reply; 5+ messages in thread
From: Chris Wilson @ 2018-06-22  7:12 UTC (permalink / raw)
  To: Tarun Vyas, intel-gfx; +Cc: dhinakaran.pandiyan, rodrigo.vivi

Quoting Tarun Vyas (2018-06-22 08:05:21)
> This is a lockless version of the exisiting psr_wait_for_idle().
> We want to wait for PSR to idle out inside intel_pipe_update_start.
> At the time of a pipe update, we should never race with any psr
> enable or disable code, which is a part of crtc enable/disable. So,
> we can live w/o taking any psr locks at all.
> The follow up patch will use this lockless wait inside pipe_update_
> start to wait for PSR to idle out before checking for vblank evasion.
> 
> Even if psr is never enabled, psr2_enabled will be false and this
> function will wait for PSR1 to idle out, which should just return
> immediately, so a very short (~1-2 usec) wait for cases where PSR
> is disabled.
> 
> v2: Add comment to explain the 25msec timeout (DK)
> 
> Signed-off-by: Tarun Vyas <tarun.vyas@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_drv.h |  1 +
>  drivers/gpu/drm/i915/intel_psr.c | 24 ++++++++++++++++++++++++
>  2 files changed, 25 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 578346b8d7e2..a48aad0f99bf 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1920,6 +1920,7 @@ void intel_psr_compute_config(struct intel_dp *intel_dp,
>                               struct intel_crtc_state *crtc_state);
>  void intel_psr_irq_control(struct drm_i915_private *dev_priv, bool debug);
>  void intel_psr_irq_handler(struct drm_i915_private *dev_priv, u32 psr_iir);
> +void psr_wait_for_idle_lockless(struct drm_i915_private *dev_priv);
>  
>  /* intel_runtime_pm.c */
>  int intel_power_domains_init(struct drm_i915_private *);
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index aea81ace854b..8e69e6193063 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -757,6 +757,30 @@ void intel_psr_disable(struct intel_dp *intel_dp,
>         cancel_work_sync(&dev_priv->psr.work);
>  }
>  
> +void psr_wait_for_idle_lockless(struct drm_i915_private *dev_priv)

intel_psr_

No need for lockless here you don't export anything else. Rename one of
the two to avoid the conflict. Probably __psr_wait_for_idle_locked,
though you would expect some sort of reuse between very similarly named
functions.

> +{
> +       i915_reg_t reg;
> +       u32 mask;
> +       int err;
> +
> +       if (dev_priv->psr.psr2_enabled) {
> +               reg = EDP_PSR2_STATUS;
> +               mask = EDP_PSR2_STATUS_STATE_MASK;
> +       } else {
> +               reg = EDP_PSR_STATUS;
> +               mask = EDP_PSR_STATUS_STATE_MASK;
> +       }
> +
> +       /*
> +        * The  25 msec timeout accounts for a frame @ 60Hz refresh rate,
> +        * exit training an aux handshake time.
> +        */
> +       err = intel_wait_for_register(dev_priv, reg, mask,
> +                                     EDP_PSR_STATUS_STATE_IDLE, 25);
> +       if (err)
> +               DRM_ERROR("Timed out waiting for PSR Idle for pipe update\n");

Propagate the error, let the caller decide if it's an error or not.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH v2] drm/i915/psr: Lockless version of psr_wait_for_idle
  2018-06-22  7:12 ` Chris Wilson
@ 2018-06-22  7:26   ` Tarun Vyas
  0 siblings, 0 replies; 5+ messages in thread
From: Tarun Vyas @ 2018-06-22  7:26 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx, dhinakaran.pandiyan, rodrigo.vivi

On Fri, Jun 22, 2018 at 08:12:00AM +0100, Chris Wilson wrote:
> Quoting Tarun Vyas (2018-06-22 08:05:21)
> > This is a lockless version of the exisiting psr_wait_for_idle().
> > We want to wait for PSR to idle out inside intel_pipe_update_start.
> > At the time of a pipe update, we should never race with any psr
> > enable or disable code, which is a part of crtc enable/disable. So,
> > we can live w/o taking any psr locks at all.
> > The follow up patch will use this lockless wait inside pipe_update_
> > start to wait for PSR to idle out before checking for vblank evasion.
> > 
> > Even if psr is never enabled, psr2_enabled will be false and this
> > function will wait for PSR1 to idle out, which should just return
> > immediately, so a very short (~1-2 usec) wait for cases where PSR
> > is disabled.
> > 
> > v2: Add comment to explain the 25msec timeout (DK)
> > 
> > Signed-off-by: Tarun Vyas <tarun.vyas@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_drv.h |  1 +
> >  drivers/gpu/drm/i915/intel_psr.c | 24 ++++++++++++++++++++++++
> >  2 files changed, 25 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index 578346b8d7e2..a48aad0f99bf 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1920,6 +1920,7 @@ void intel_psr_compute_config(struct intel_dp *intel_dp,
> >                               struct intel_crtc_state *crtc_state);
> >  void intel_psr_irq_control(struct drm_i915_private *dev_priv, bool debug);
> >  void intel_psr_irq_handler(struct drm_i915_private *dev_priv, u32 psr_iir);
> > +void psr_wait_for_idle_lockless(struct drm_i915_private *dev_priv);
> >  
> >  /* intel_runtime_pm.c */
> >  int intel_power_domains_init(struct drm_i915_private *);
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> > index aea81ace854b..8e69e6193063 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -757,6 +757,30 @@ void intel_psr_disable(struct intel_dp *intel_dp,
> >         cancel_work_sync(&dev_priv->psr.work);
> >  }
> >  
> > +void psr_wait_for_idle_lockless(struct drm_i915_private *dev_priv)
> 
> intel_psr_
> 
> No need for lockless here you don't export anything else. Rename one of
> the two to avoid the conflict. Probably __psr_wait_for_idle_locked,
> though you would expect some sort of reuse between very similarly named
> functions.
>
Thanks for the review Chris.

Yes, there is a lot duplication between the two, but the locking drop in psr_wait_for_idle(), before the wait_for_register is making it harder to reuse it.
Will rename the original wait function.  
> > +{
> > +       i915_reg_t reg;
> > +       u32 mask;
> > +       int err;
> > +
> > +       if (dev_priv->psr.psr2_enabled) {
> > +               reg = EDP_PSR2_STATUS;
> > +               mask = EDP_PSR2_STATUS_STATE_MASK;
> > +       } else {
> > +               reg = EDP_PSR_STATUS;
> > +               mask = EDP_PSR_STATUS_STATE_MASK;
> > +       }
> > +
> > +       /*
> > +        * The  25 msec timeout accounts for a frame @ 60Hz refresh rate,
> > +        * exit training an aux handshake time.
> > +        */
> > +       err = intel_wait_for_register(dev_priv, reg, mask,
> > +                                     EDP_PSR_STATUS_STATE_IDLE, 25);
> > +       if (err)
> > +               DRM_ERROR("Timed out waiting for PSR Idle for pipe update\n");
> 
> Propagate the error, let the caller decide if it's an error or not.
> -Chrisa
Will do, in the next version.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* ✓ Fi.CI.BAT: success for drm/i915/psr: Lockless version of psr_wait_for_idle (rev3)
  2018-06-22  7:05 [PATCH v2] drm/i915/psr: Lockless version of psr_wait_for_idle Tarun Vyas
  2018-06-22  7:12 ` Chris Wilson
@ 2018-06-22  7:39 ` Patchwork
  2018-06-22 14:24 ` ✓ Fi.CI.IGT: " Patchwork
  2 siblings, 0 replies; 5+ messages in thread
From: Patchwork @ 2018-06-22  7:39 UTC (permalink / raw)
  To: Tarun Vyas; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/psr: Lockless version of psr_wait_for_idle (rev3)
URL   : https://patchwork.freedesktop.org/series/45209/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4368 -> Patchwork_9394 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/45209/revisions/3/mbox/

== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

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

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


== Participating hosts (43 -> 37) ==

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


== Build changes ==

    * Linux: CI_DRM_4368 -> Patchwork_9394

  CI_DRM_4368: f9f621dc095a8bfd2157fba018ddb542260d8daa @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4528: 6be300d405de5974b262e8b93a445be4ac618e6a @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9394: ac5d6f358698e977137063905a20ca8c19a090b1 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

ac5d6f358698 drm/i915/psr: Lockless version of psr_wait_for_idle

== Logs ==

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

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

* ✓ Fi.CI.IGT: success for drm/i915/psr: Lockless version of psr_wait_for_idle (rev3)
  2018-06-22  7:05 [PATCH v2] drm/i915/psr: Lockless version of psr_wait_for_idle Tarun Vyas
  2018-06-22  7:12 ` Chris Wilson
  2018-06-22  7:39 ` ✓ Fi.CI.BAT: success for drm/i915/psr: Lockless version of psr_wait_for_idle (rev3) Patchwork
@ 2018-06-22 14:24 ` Patchwork
  2 siblings, 0 replies; 5+ messages in thread
From: Patchwork @ 2018-06-22 14:24 UTC (permalink / raw)
  To: Tarun Vyas; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/psr: Lockless version of psr_wait_for_idle (rev3)
URL   : https://patchwork.freedesktop.org/series/45209/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4368_full -> Patchwork_9394_full =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_9394_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_9394_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_9394_full:

  === IGT changes ===

    ==== Warnings ====

    igt@drv_selftest@live_guc:
      shard-kbl:          PASS -> SKIP +1

    igt@gem_mocs_settings@mocs-rc6-bsd1:
      shard-kbl:          SKIP -> PASS +1

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

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

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

    
    ==== Possible fixes ====

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

    igt@kms_flip@2x-plain-flip-ts-check-interruptible:
      shard-glk:          FAIL (fdo#100368) -> PASS

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

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

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
      shard-apl:          FAIL (fdo#103375) -> PASS

    igt@perf_pmu@busy-idle-check-all-rcs0:
      shard-snb:          INCOMPLETE (fdo#105411) -> PASS

    
    ==== Warnings ====

    igt@drv_selftest@live_gtt:
      shard-glk:          INCOMPLETE (k.org#198133, fdo#103359) -> FAIL (fdo#105347)

    
  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
  fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
  fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375
  fdo#103822 https://bugs.freedesktop.org/show_bug.cgi?id=103822
  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#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411
  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
  k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133


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

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4368 -> Patchwork_9394

  CI_DRM_4368: f9f621dc095a8bfd2157fba018ddb542260d8daa @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4528: 6be300d405de5974b262e8b93a445be4ac618e6a @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9394: ac5d6f358698e977137063905a20ca8c19a090b1 @ 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_9394/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

end of thread, other threads:[~2018-06-22 14:24 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-06-22  7:05 [PATCH v2] drm/i915/psr: Lockless version of psr_wait_for_idle Tarun Vyas
2018-06-22  7:12 ` Chris Wilson
2018-06-22  7:26   ` Tarun Vyas
2018-06-22  7:39 ` ✓ Fi.CI.BAT: success for drm/i915/psr: Lockless version of psr_wait_for_idle (rev3) Patchwork
2018-06-22 14:24 ` ✓ Fi.CI.IGT: " Patchwork

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox