* [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