Re: [Intel-gfx] [PATCH 1/1] drm/i915: Do not enable PSR2/selective fetch if there are no planes
On Tue, Jun 14, 2022 at 03:55:04PM +0300, Hogander, Jouni wrote:
> On Tue, 2022-06-14 at 15:22 +0300, Stanislav Lisovskiy wrote:
> > We seem to enable PSR2 and selective fetch even if there are no
> > active
> > planes. That seems to causes FIFO underruns at least for ADLP.
> > Those are gone if we don't do that. Just adding simple check
> > in intel_psr2_sel_fetch_config_valid seems to do the trick.
>
> We are already disabling PSR intel_psr_pre_plane_update if
> active_planes is 0.
>
> We are also checking active_planes in _intel_psr_post_plane_update and
> not enabling PSR if it's 0.
>
> So I'm now wondering what sequence this patch is actually changing?
> I.e. where PSR is currently enabled/not disabled if active_planes == 0?
>
> > Signed-off-by: Stanislav Lisovskiy
> > ---
> > drivers/gpu/drm/i915/display/intel_psr.c | 6 ++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > b/drivers/gpu/drm/i915/display/intel_psr.c
> > index 7d61c55184e5..03add69cfdca 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -747,6 +747,12 @@ static bool
> > intel_psr2_sel_fetch_config_valid(struct intel_dp *intel_dp,
> > return false;
> > }
> >
> > + if (hweight32(crtc_state->active_planes) == 0) {
> > + drm_dbg_kms(_priv->drm,
> > + "PSR2 sel fetch not enabled, no
> > active_planes\n");
> > + return false;
> > + }
> > +
> > /* Wa_14010254185 Wa_14010103792 */
> > if (IS_TGL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_C0)) {
> > drm_dbg_kms(_priv->drm,
>
Added dump_stack to that condition as mentioned in prev mail, here's what we
have:
[ 258.438169] i915 :00:02.0: [drm:intel_psr_compute_config [i915]] PSR2
sel fetch not enabled, no active planes
--
[ 258.441108] Call Trace:
[ 258.441108]
[ 258.441109] dump_stack_lvl+0x56/0x7b
[ 258.44] intel_psr_compute_config+0x7a8/0x900 [i915]
[ 258.441170] intel_dp_compute_config+0x21a/0x700 [i915]
[ 258.441227] intel_ddi_compute_config+0x8c/0xc0 [i915]
[ 258.441284] intel_atomic_check+0x165a/0x3090 [i915]
[ 258.441344] ? drm_atomic_check_only+0x39/0xa60
[ 258.441350] drm_atomic_check_only+0x64f/0xa60
[ 258.441354] drm_atomic_commit+0x51/0xc0
[ 258.441355] ? __drm_printfn_seq_file+0x20/0x20
[ 258.441358] drm_mode_atomic_ioctl+0x890/0xa30
[ 258.441369] ? drm_atomic_set_property+0xa80/0xa80
[ 258.441371] drm_ioctl_kernel+0xb2/0x140
[ 258.441374] drm_ioctl+0x316/0x3e0
[ 258.441377] ? drm_atomic_set_property+0xa80/0xa80
[ 258.441381] ? find_held_lock+0x2d/0x90
[ 258.441386] __x64_sys_ioctl+0x6e/0xb0
[ 258.441387] ? lockdep_hardirqs_on+0xbf/0x130
[ 258.441389] do_syscall_64+0x37/0x80
[ 258.441390] entry_SYSCALL_64_after_hwframe+0x46/0xb0
[ 258.441391] RIP: 0033:0x7fd3e88e231b
[ 258.441392] Code: 89 d8 49 8d 3c 1c 48 f7 d8 49 39 c4 72 b5 e8 1c ff ff ff
85 c0 78 ba 4c 89 e0 5b 5d 41 5c c3 f3 0f 1e fa b8 10 00 00 00 0f 05 <48> 3d 01
f0 ff ff 73 01 c3 48 8b 0d 1d 3b 0d 00 f7 d8 64 89 01 48
[ 258.441393] RSP: 002b:7ffc4a4c9008 EFLAGS: 0246 ORIG_RAX:
0010
[ 258.441394] RAX: ffda RBX: 7ffc4a4c9050 RCX: 7fd3e88e231b
[ 258.441394] RDX: 7ffc4a4c9050 RSI: c03864bc RDI: 0003
[ 258.441395] RBP: c03864bc R08: 0002 R09: 0002
[ 258.441395] R10: 0007 R11: 0246 R12: 56007e393620
[ 258.441396] R13: 0003 R14: 56007e2dbb10 R15: 56007e386f00
[ 258.441402]
Stan
Re: [Intel-gfx] [PATCH 1/1] drm/i915: Do not enable PSR2/selective fetch if there are no planes
On Tue, Jun 14, 2022 at 03:55:04PM +0300, Hogander, Jouni wrote:
> On Tue, 2022-06-14 at 15:22 +0300, Stanislav Lisovskiy wrote:
> > We seem to enable PSR2 and selective fetch even if there are no
> > active
> > planes. That seems to causes FIFO underruns at least for ADLP.
> > Those are gone if we don't do that. Just adding simple check
> > in intel_psr2_sel_fetch_config_valid seems to do the trick.
>
> We are already disabling PSR intel_psr_pre_plane_update if
> active_planes is 0.
>
> We are also checking active_planes in _intel_psr_post_plane_update and
> not enabling PSR if it's 0.
>
> So I'm now wondering what sequence this patch is actually changing?
> I.e. where PSR is currently enabled/not disabled if active_planes == 0?
Good question! Apparently we still do it, because without this change
we get FIFO underruns, while with that one we don't.
I have suspicion that this happens during modeset, however you are right
we need to know for sure.
I will get back here and post the exact call trace.
One thing I can say for sure that we do it somewhere, otherwise adding
this wouldn't have any effect.
Stan
>
> > Signed-off-by: Stanislav Lisovskiy
> > ---
> > drivers/gpu/drm/i915/display/intel_psr.c | 6 ++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > b/drivers/gpu/drm/i915/display/intel_psr.c
> > index 7d61c55184e5..03add69cfdca 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -747,6 +747,12 @@ static bool
> > intel_psr2_sel_fetch_config_valid(struct intel_dp *intel_dp,
> > return false;
> > }
> >
> > + if (hweight32(crtc_state->active_planes) == 0) {
> > + drm_dbg_kms(_priv->drm,
> > + "PSR2 sel fetch not enabled, no
> > active_planes\n");
> > + return false;
> > + }
> > +
> > /* Wa_14010254185 Wa_14010103792 */
> > if (IS_TGL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_C0)) {
> > drm_dbg_kms(_priv->drm,
>
Re: [Intel-gfx] [PATCH 1/1] drm/i915: Do not enable PSR2/selective fetch if there are no planes
On Tue, 2022-06-14 at 15:22 +0300, Stanislav Lisovskiy wrote:
> We seem to enable PSR2 and selective fetch even if there are no
> active
> planes. That seems to causes FIFO underruns at least for ADLP.
> Those are gone if we don't do that. Just adding simple check
> in intel_psr2_sel_fetch_config_valid seems to do the trick.
We are already disabling PSR intel_psr_pre_plane_update if
active_planes is 0.
We are also checking active_planes in _intel_psr_post_plane_update and
not enabling PSR if it's 0.
So I'm now wondering what sequence this patch is actually changing?
I.e. where PSR is currently enabled/not disabled if active_planes == 0?
> Signed-off-by: Stanislav Lisovskiy
> ---
> drivers/gpu/drm/i915/display/intel_psr.c | 6 ++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> b/drivers/gpu/drm/i915/display/intel_psr.c
> index 7d61c55184e5..03add69cfdca 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -747,6 +747,12 @@ static bool
> intel_psr2_sel_fetch_config_valid(struct intel_dp *intel_dp,
> return false;
> }
>
> + if (hweight32(crtc_state->active_planes) == 0) {
> + drm_dbg_kms(_priv->drm,
> + "PSR2 sel fetch not enabled, no
> active_planes\n");
> + return false;
> + }
> +
> /* Wa_14010254185 Wa_14010103792 */
> if (IS_TGL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_C0)) {
> drm_dbg_kms(_priv->drm,
[Intel-gfx] [PATCH 1/1] drm/i915: Do not enable PSR2/selective fetch if there are no planes
We seem to enable PSR2 and selective fetch even if there are no active
planes. That seems to causes FIFO underruns at least for ADLP.
Those are gone if we don't do that. Just adding simple check
in intel_psr2_sel_fetch_config_valid seems to do the trick.
Signed-off-by: Stanislav Lisovskiy
---
drivers/gpu/drm/i915/display/intel_psr.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
b/drivers/gpu/drm/i915/display/intel_psr.c
index 7d61c55184e5..03add69cfdca 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -747,6 +747,12 @@ static bool intel_psr2_sel_fetch_config_valid(struct
intel_dp *intel_dp,
return false;
}
+ if (hweight32(crtc_state->active_planes) == 0) {
+ drm_dbg_kms(_priv->drm,
+ "PSR2 sel fetch not enabled, no active_planes\n");
+ return false;
+ }
+
/* Wa_14010254185 Wa_14010103792 */
if (IS_TGL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_C0)) {
drm_dbg_kms(_priv->drm,
--
2.24.1.485.gad05a3d8e5
