* [PATCH 1/2] drm/i915/psr: Remove wait_for_idle() for PSR2
@ 2018-08-24 23:08 Dhinakaran Pandiyan
2018-08-24 23:08 ` [PATCH 2/2] drm/i915/psr: Rewrite comments in intel_psr_wait_for_idle() Dhinakaran Pandiyan
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Dhinakaran Pandiyan @ 2018-08-24 23:08 UTC (permalink / raw)
To: intel-gfx; +Cc: Dhinakaran Pandiyan, Rodrigo Vivi
CI runs show PSR2 does not go to IDLE with selective update enabled on
all PSR exit triggers. Specifically, logs indicate the hardware enters
"SLEEP Selective Update" and not "IDLE Reset state', like the kernel
expects, when vblank interrupts are enabled. This check was added for PSR1
but incorrectly extended to PSR2, remove the check as it breaks tests
and prints out misleading error messages.
v2: Split out non-code changes (Rodrigo)
Cc: Tarun Vyas <tarun.vyas@intel.com>
Cc: José Roberto de Souza <jose.souza@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Fixes: c43dbcbbcc8c ("drm/i915/psr: Lockless version of psr_wait_for_idle")
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
drivers/gpu/drm/i915/intel_psr.c | 16 ++++++----------
1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index da583a45e942..2cb931f3019b 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -771,8 +771,6 @@ int intel_psr_wait_for_idle(const struct intel_crtc_state *new_crtc_state,
{
struct intel_crtc *crtc = to_intel_crtc(new_crtc_state->base.crtc);
struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
- i915_reg_t reg;
- u32 mask;
if (!dev_priv->psr.enabled || !new_crtc_state->has_psr)
return 0;
@@ -787,13 +785,10 @@ int intel_psr_wait_for_idle(const struct intel_crtc_state *new_crtc_state,
* not needed and will induce latencies in the atomic
* update path.
*/
- 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;
- }
+
+ /* FIXME: Update this for PSR2 if we need to wait for idle */
+ if (READ_ONCE(dev_priv->psr.psr2_enabled))
+ return 0;
/*
* Max time for PSR to idle = Inverse of the refresh rate +
@@ -801,7 +796,8 @@ int intel_psr_wait_for_idle(const struct intel_crtc_state *new_crtc_state,
* handshake. 50 msec is defesive enough to cover everything.
*/
- return __intel_wait_for_register(dev_priv, reg, mask,
+ return __intel_wait_for_register(dev_priv, EDP_PSR_STATUS,
+ EDP_PSR_STATUS_STATE_MASK,
EDP_PSR_STATUS_STATE_IDLE, 2, 50,
out_value);
}
--
2.17.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/2] drm/i915/psr: Rewrite comments in intel_psr_wait_for_idle()
2018-08-24 23:08 [PATCH 1/2] drm/i915/psr: Remove wait_for_idle() for PSR2 Dhinakaran Pandiyan
@ 2018-08-24 23:08 ` Dhinakaran Pandiyan
2018-08-24 23:19 ` Rodrigo Vivi
2018-08-24 23:18 ` [PATCH 1/2] drm/i915/psr: Remove wait_for_idle() for PSR2 Rodrigo Vivi
` (2 subsequent siblings)
3 siblings, 1 reply; 6+ messages in thread
From: Dhinakaran Pandiyan @ 2018-08-24 23:08 UTC (permalink / raw)
To: intel-gfx; +Cc: Dhinakaran Pandiyan, Rodrigo Vivi
Added bspec reference, aligned text and documented the function.
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
drivers/gpu/drm/i915/intel_psr.c | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 2cb931f3019b..aee64aee18fe 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -766,6 +766,16 @@ void intel_psr_disable(struct intel_dp *intel_dp,
cancel_work_sync(&dev_priv->psr.work);
}
+/**
+ * intel_psr_wait_for_idle - wait for PSR1 to idle
+ * @new_crtc_state: new CRTC state
+ * @out_value: PSR status in case of failure
+ *
+ * This function is expected to be called from pipe_update_start() where it is
+ * not expected to race with PSR enable or disable.
+ *
+ * Returns: 0 on success or -ETIMEOUT if PSR status does not idle.
+ */
int intel_psr_wait_for_idle(const struct intel_crtc_state *new_crtc_state,
u32 *out_value)
{
@@ -775,25 +785,15 @@ int intel_psr_wait_for_idle(const struct intel_crtc_state *new_crtc_state,
if (!dev_priv->psr.enabled || !new_crtc_state->has_psr)
return 0;
- /*
- * The sole user right now is intel_pipe_update_start(),
- * which won't race with psr_enable/disable, which is
- * where psr2_enabled is written to. So, we don't need
- * to acquire the psr.lock. More importantly, we want the
- * latency inside intel_pipe_update_start() to be as low
- * as possible, so no need to acquire psr.lock when it is
- * not needed and will induce latencies in the atomic
- * update path.
- */
-
/* FIXME: Update this for PSR2 if we need to wait for idle */
if (READ_ONCE(dev_priv->psr.psr2_enabled))
return 0;
/*
- * Max time for PSR to idle = Inverse of the refresh rate +
- * 6 ms of exit training time + 1.5 ms of aux channel
- * handshake. 50 msec is defesive enough to cover everything.
+ * From bspec: Panel Self Refresh (BDW+)
+ * Max. time for PSR to idle = Inverse of the refresh rate + 6 ms of
+ * exit training time + 1.5 ms of aux channel handshake. 50 ms is
+ * defensive enough to cover everything.
*/
return __intel_wait_for_register(dev_priv, EDP_PSR_STATUS,
--
2.17.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 2/2] drm/i915/psr: Rewrite comments in intel_psr_wait_for_idle()
2018-08-24 23:08 ` [PATCH 2/2] drm/i915/psr: Rewrite comments in intel_psr_wait_for_idle() Dhinakaran Pandiyan
@ 2018-08-24 23:19 ` Rodrigo Vivi
0 siblings, 0 replies; 6+ messages in thread
From: Rodrigo Vivi @ 2018-08-24 23:19 UTC (permalink / raw)
To: Dhinakaran Pandiyan; +Cc: intel-gfx
On Fri, Aug 24, 2018 at 04:08:44PM -0700, Dhinakaran Pandiyan wrote:
> Added bspec reference, aligned text and documented the function.
>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
> drivers/gpu/drm/i915/intel_psr.c | 28 ++++++++++++++--------------
> 1 file changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index 2cb931f3019b..aee64aee18fe 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -766,6 +766,16 @@ void intel_psr_disable(struct intel_dp *intel_dp,
> cancel_work_sync(&dev_priv->psr.work);
> }
>
> +/**
> + * intel_psr_wait_for_idle - wait for PSR1 to idle
> + * @new_crtc_state: new CRTC state
> + * @out_value: PSR status in case of failure
> + *
> + * This function is expected to be called from pipe_update_start() where it is
> + * not expected to race with PSR enable or disable.
> + *
> + * Returns: 0 on success or -ETIMEOUT if PSR status does not idle.
> + */
> int intel_psr_wait_for_idle(const struct intel_crtc_state *new_crtc_state,
> u32 *out_value)
> {
> @@ -775,25 +785,15 @@ int intel_psr_wait_for_idle(const struct intel_crtc_state *new_crtc_state,
> if (!dev_priv->psr.enabled || !new_crtc_state->has_psr)
> return 0;
>
> - /*
> - * The sole user right now is intel_pipe_update_start(),
> - * which won't race with psr_enable/disable, which is
> - * where psr2_enabled is written to. So, we don't need
> - * to acquire the psr.lock. More importantly, we want the
> - * latency inside intel_pipe_update_start() to be as low
> - * as possible, so no need to acquire psr.lock when it is
> - * not needed and will induce latencies in the atomic
> - * update path.
> - */
> -
> /* FIXME: Update this for PSR2 if we need to wait for idle */
> if (READ_ONCE(dev_priv->psr.psr2_enabled))
> return 0;
>
> /*
> - * Max time for PSR to idle = Inverse of the refresh rate +
> - * 6 ms of exit training time + 1.5 ms of aux channel
> - * handshake. 50 msec is defesive enough to cover everything.
> + * From bspec: Panel Self Refresh (BDW+)
> + * Max. time for PSR to idle = Inverse of the refresh rate + 6 ms of
> + * exit training time + 1.5 ms of aux channel handshake. 50 ms is
> + * defensive enough to cover everything.
> */
>
> return __intel_wait_for_register(dev_priv, EDP_PSR_STATUS,
> --
> 2.17.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] 6+ messages in thread
* Re: [PATCH 1/2] drm/i915/psr: Remove wait_for_idle() for PSR2
2018-08-24 23:08 [PATCH 1/2] drm/i915/psr: Remove wait_for_idle() for PSR2 Dhinakaran Pandiyan
2018-08-24 23:08 ` [PATCH 2/2] drm/i915/psr: Rewrite comments in intel_psr_wait_for_idle() Dhinakaran Pandiyan
@ 2018-08-24 23:18 ` Rodrigo Vivi
2018-08-24 23:31 ` ✓ Fi.CI.BAT: success for series starting with [1/2] " Patchwork
2018-08-25 0:20 ` ✓ Fi.CI.IGT: " Patchwork
3 siblings, 0 replies; 6+ messages in thread
From: Rodrigo Vivi @ 2018-08-24 23:18 UTC (permalink / raw)
To: Dhinakaran Pandiyan; +Cc: intel-gfx
On Fri, Aug 24, 2018 at 04:08:43PM -0700, Dhinakaran Pandiyan wrote:
> CI runs show PSR2 does not go to IDLE with selective update enabled on
> all PSR exit triggers. Specifically, logs indicate the hardware enters
> "SLEEP Selective Update" and not "IDLE Reset state', like the kernel
> expects, when vblank interrupts are enabled. This check was added for PSR1
> but incorrectly extended to PSR2, remove the check as it breaks tests
> and prints out misleading error messages.
>
> v2: Split out non-code changes (Rodrigo)
>
> Cc: Tarun Vyas <tarun.vyas@intel.com>
> Cc: José Roberto de Souza <jose.souza@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Fixes: c43dbcbbcc8c ("drm/i915/psr: Lockless version of psr_wait_for_idle")
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
> drivers/gpu/drm/i915/intel_psr.c | 16 ++++++----------
> 1 file changed, 6 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index da583a45e942..2cb931f3019b 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -771,8 +771,6 @@ int intel_psr_wait_for_idle(const struct intel_crtc_state *new_crtc_state,
> {
> struct intel_crtc *crtc = to_intel_crtc(new_crtc_state->base.crtc);
> struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> - i915_reg_t reg;
> - u32 mask;
>
> if (!dev_priv->psr.enabled || !new_crtc_state->has_psr)
> return 0;
> @@ -787,13 +785,10 @@ int intel_psr_wait_for_idle(const struct intel_crtc_state *new_crtc_state,
> * not needed and will induce latencies in the atomic
> * update path.
> */
> - 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;
> - }
> +
> + /* FIXME: Update this for PSR2 if we need to wait for idle */
> + if (READ_ONCE(dev_priv->psr.psr2_enabled))
> + return 0;
>
> /*
> * Max time for PSR to idle = Inverse of the refresh rate +
> @@ -801,7 +796,8 @@ int intel_psr_wait_for_idle(const struct intel_crtc_state *new_crtc_state,
> * handshake. 50 msec is defesive enough to cover everything.
> */
>
> - return __intel_wait_for_register(dev_priv, reg, mask,
> + return __intel_wait_for_register(dev_priv, EDP_PSR_STATUS,
> + EDP_PSR_STATUS_STATE_MASK,
> EDP_PSR_STATUS_STATE_IDLE, 2, 50,
> out_value);
> }
> --
> 2.17.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] 6+ messages in thread* ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915/psr: Remove wait_for_idle() for PSR2
2018-08-24 23:08 [PATCH 1/2] drm/i915/psr: Remove wait_for_idle() for PSR2 Dhinakaran Pandiyan
2018-08-24 23:08 ` [PATCH 2/2] drm/i915/psr: Rewrite comments in intel_psr_wait_for_idle() Dhinakaran Pandiyan
2018-08-24 23:18 ` [PATCH 1/2] drm/i915/psr: Remove wait_for_idle() for PSR2 Rodrigo Vivi
@ 2018-08-24 23:31 ` Patchwork
2018-08-25 0:20 ` ✓ Fi.CI.IGT: " Patchwork
3 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2018-08-24 23:31 UTC (permalink / raw)
To: Dhinakaran Pandiyan; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/2] drm/i915/psr: Remove wait_for_idle() for PSR2
URL : https://patchwork.freedesktop.org/series/48691/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4704 -> Patchwork_10014 =
== Summary - SUCCESS ==
No regressions found.
External URL: https://patchwork.freedesktop.org/api/1.0/series/48691/revisions/1/mbox/
== Possible new issues ==
Here are the unknown changes that may have been introduced in Patchwork_10014:
=== IGT changes ===
==== Warnings ====
{igt@kms_psr@cursor_plane_move}:
fi-cnl-psr: DMESG-FAIL -> FAIL
== Known issues ==
Here are the changes found in Patchwork_10014 that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@drv_selftest@live_hangcheck:
{fi-cfl-8109u}: PASS -> DMESG-FAIL (fdo#106560)
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
fi-bxt-dsi: PASS -> INCOMPLETE (fdo#103927)
==== Possible fixes ====
igt@kms_pipe_crc_basic@hang-read-crc-pipe-b:
fi-skl-guc: FAIL (fdo#103191) -> PASS +1
{fi-byt-clapper}: FAIL (fdo#107362, fdo#103191) -> PASS
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
fi-skl-6260u: INCOMPLETE (fdo#104108) -> PASS
{igt@kms_psr@primary_mmap_gtt}:
fi-cnl-psr: DMESG-WARN -> PASS +1
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
fdo#104108 https://bugs.freedesktop.org/show_bug.cgi?id=104108
fdo#106560 https://bugs.freedesktop.org/show_bug.cgi?id=106560
fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
== Participating hosts (54 -> 48) ==
Missing (6): fi-ilk-m540 fi-hsw-4200u fi-byt-j1900 fi-byt-squawks fi-bsw-cyan fi-ctg-p8600
== Build changes ==
* Linux: CI_DRM_4704 -> Patchwork_10014
CI_DRM_4704: eb9a20b20d4790be1561235f45e209fba02dc6c0 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4609: 0bc9763af77bbb37f2ed65cc39c398e88db7d8e3 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_10014: 45eabe700f63f7160911b083c0775e251871bc0e @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
45eabe700f63 drm/i915/psr: Rewrite comments in intel_psr_wait_for_idle()
62024ccae1a4 drm/i915/psr: Remove wait_for_idle() for PSR2
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10014/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread* ✓ Fi.CI.IGT: success for series starting with [1/2] drm/i915/psr: Remove wait_for_idle() for PSR2
2018-08-24 23:08 [PATCH 1/2] drm/i915/psr: Remove wait_for_idle() for PSR2 Dhinakaran Pandiyan
` (2 preceding siblings ...)
2018-08-24 23:31 ` ✓ Fi.CI.BAT: success for series starting with [1/2] " Patchwork
@ 2018-08-25 0:20 ` Patchwork
3 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2018-08-25 0:20 UTC (permalink / raw)
To: Pandiyan, Dhinakaran; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/2] drm/i915/psr: Remove wait_for_idle() for PSR2
URL : https://patchwork.freedesktop.org/series/48691/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4704_full -> Patchwork_10014_full =
== Summary - SUCCESS ==
No regressions found.
== Known issues ==
Here are the changes found in Patchwork_10014_full that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@gem_ctx_isolation@bcs0-s3:
shard-kbl: PASS -> INCOMPLETE (fdo#103665) +1
igt@kms_flip@flip-vs-expired-vblank-interruptible:
shard-glk: PASS -> FAIL (fdo#105363)
igt@kms_setmode@basic:
shard-kbl: PASS -> FAIL (fdo#99912)
==== Possible fixes ====
igt@kms_cursor_legacy@cursor-vs-flip-toggle:
shard-hsw: FAIL (fdo#103355) -> PASS
igt@kms_setmode@basic:
shard-apl: FAIL (fdo#99912) -> PASS
igt@kms_vblank@pipe-a-ts-continuation-suspend:
shard-hsw: FAIL (fdo#104894) -> PASS
fdo#103355 https://bugs.freedesktop.org/show_bug.cgi?id=103355
fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
fdo#104894 https://bugs.freedesktop.org/show_bug.cgi?id=104894
fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
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_4704 -> Patchwork_10014
CI_DRM_4704: eb9a20b20d4790be1561235f45e209fba02dc6c0 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4609: 0bc9763af77bbb37f2ed65cc39c398e88db7d8e3 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_10014: 45eabe700f63f7160911b083c0775e251871bc0e @ 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_10014/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-08-25 0:20 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-08-24 23:08 [PATCH 1/2] drm/i915/psr: Remove wait_for_idle() for PSR2 Dhinakaran Pandiyan
2018-08-24 23:08 ` [PATCH 2/2] drm/i915/psr: Rewrite comments in intel_psr_wait_for_idle() Dhinakaran Pandiyan
2018-08-24 23:19 ` Rodrigo Vivi
2018-08-24 23:18 ` [PATCH 1/2] drm/i915/psr: Remove wait_for_idle() for PSR2 Rodrigo Vivi
2018-08-24 23:31 ` ✓ Fi.CI.BAT: success for series starting with [1/2] " Patchwork
2018-08-25 0:20 ` ✓ 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.