* [PATCH v11 0/1] drm/i915/display: Add no_psr_reason to PSR debugfs @ 2025-09-15 14:19 Michał Grzelak 2025-09-15 14:19 ` [PATCH v11 1/1] " Michał Grzelak 0 siblings, 1 reply; 4+ messages in thread From: Michał Grzelak @ 2025-09-15 14:19 UTC (permalink / raw) To: jouni.hogander, intel-gfx, intel-xe Cc: sebastian.brzezinka, Michał Grzelak Next version of v10 [1]. Took into consideration Jouni's review [2]. IGT patch associated with this patch: [3]. [1] https://lore.kernel.org/intel-gfx/20250903103152.2440348-1-michal.grzelak@intel.com [2] https://lore.kernel.org/intel-gfx/ad14c2fd3d8e505f835cdb59daf1cbff5fb3ed9c.camel@intel.com [3] https://lore.kernel.org/igt-dev/20250828104311.1279788-1-michal.grzelak@intel.com Test-with: 20250828104311.1279788-2-michal.grzelak@intel.com Best regards, Michał --- Changelog: v10->v11 - update the reason from new_crtc_state instead of old_crtc_state [Jouni] v9->v10 - log reason into intel_crtc_state instead of intel_dp->psr [Jouni] - remove clearing no_psr_reason in intel_psr_compute_config [Jouni] - change update of no_psr_reason into more readable form [Jouni] v8->v9 - add no_psr_reason into struct intel_crtc_state [Jouni] - update the reason in intel_psr_pre_plane_update [Jouni] - elaborate on motivation of the feature in commit message [Sebastian] - copy changelog to commit message [Sebastian] v7->v8 - reset no_psr_reason at the begin of intel_psr_compute_config [Jouni] - restore keep_disabled [Jouni] - drop setting "Sink not reliable" [Jouni] - add WA number [Jouni] - if non-NULL, write no_psr_reason after PSR mode [Jouni] v6->v7 - rebase onto new drm-tip v5->v6 - move setting no_psr_reason to intel_psr_post_plane_update [Jouni] - remove setting no_psr_reason when disabling PSR is temporary [Jouni] v4->v5 - fix indentation errors from checkpatch v3->v4 - change format of logging workaround #1136 v2->v3 - change reason description to be more specific [Mika] - remove BSpecs number & WA number from being written into no_psr_reason - replace spaces with tabs v1->v2 - set other reasons than "PSR setup timing not met" - clear no_psr_reason when activating PSR. Michał Grzelak (1): drm/i915/display: Add no_psr_reason to PSR debugfs .../drm/i915/display/intel_display_types.h | 3 +++ drivers/gpu/drm/i915/display/intel_psr.c | 22 +++++++++++++++---- 2 files changed, 21 insertions(+), 4 deletions(-) -- 2.45.2 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v11 1/1] drm/i915/display: Add no_psr_reason to PSR debugfs 2025-09-15 14:19 [PATCH v11 0/1] drm/i915/display: Add no_psr_reason to PSR debugfs Michał Grzelak @ 2025-09-15 14:19 ` Michał Grzelak 2025-09-30 4:38 ` Hogander, Jouni 0 siblings, 1 reply; 4+ messages in thread From: Michał Grzelak @ 2025-09-15 14:19 UTC (permalink / raw) To: jouni.hogander, intel-gfx, intel-xe Cc: sebastian.brzezinka, Michał Grzelak There is no reason in debugfs why PSR has been disabled. Currently, without this information, IGT tests cannot decide whether PSR has been disabled on purpose or was it abnormal behavior. Because of it, the status of the test cannot be decided correctly. Add no_psr_reason field into struct intel_psr. Add no_psr_reason into struct intel_crtc_state to prevent staying out of sync when _psr_compute_config is not using computed state. Write the reason, e.g. PSR setup timing not met, into proper PSR debugfs file. Update the reason from new_crtc_state in intel_psr_pre_plane_update. Extend format of debugfs file to have reason when it is non-NULL. Clean the reason when PSR is activated. Refactor intel_psr_post_plane_update to use no_psr_reason along keep_disabled. Changelog: v10->v11 - update the reason from new_crtc_state instead of old_crtc_state [Jouni] v9->v10 - log reason into intel_crtc_state instead of intel_dp->psr [Jouni] - remove clearing no_psr_reason in intel_psr_compute_config [Jouni] - change update of no_psr_reason into more readable form [Jouni] v8->v9 - add no_psr_reason into struct intel_crtc_state [Jouni] - update the reason in intel_psr_pre_plane_update [Jouni] - elaborate on motivation of the feature in commit message [Sebastian] - copy changelog to commit message [Sebastian] v7->v8 - reset no_psr_reason at the begin of intel_psr_compute_config [Jouni] - restore keep_disabled [Jouni] - drop setting "Sink not reliable" [Jouni] - add WA number [Jouni] - if non-NULL, write no_psr_reason after PSR mode [Jouni] v6->v7 - rebase onto new drm-tip v5->v6 - move setting no_psr_reason to intel_psr_post_plane_update [Jouni] - remove setting no_psr_reason when disabling PSR is temporary [Jouni] v4->v5 - fix indentation errors from checkpatch v3->v4 - change format of logging workaround #1136 v2->v3 - change reason description to be more specific [Mika] - remove BSpecs number & WA number from being written into no_psr_reason - replace spaces with tabs v1->v2 - set other reasons than "PSR setup timing not met" - clear no_psr_reason when activating PSR. Reviewed-by: Sebastian Brzezinka <sebastian.brzezinka@intel.com> Signed-off-by: Michał Grzelak <michal.grzelak@intel.com> --- .../drm/i915/display/intel_display_types.h | 3 +++ drivers/gpu/drm/i915/display/intel_psr.c | 22 +++++++++++++++---- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h index 358ab922d7a7..8e930ec197ab 100644 --- a/drivers/gpu/drm/i915/display/intel_display_types.h +++ b/drivers/gpu/drm/i915/display/intel_display_types.h @@ -1127,6 +1127,7 @@ struct intel_crtc_state { u32 dc3co_exitline; u16 su_y_granularity; u8 active_non_psr_pipes; + const char *no_psr_reason; /* * Frequency the dpll for the port should run at. Differs from the @@ -1689,6 +1690,8 @@ struct intel_psr { bool pkg_c_latency_used; u8 active_non_psr_pipes; + + const char *no_psr_reason; }; struct intel_dp { diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c index 01bf304c705f..a1faf8a8d184 100644 --- a/drivers/gpu/drm/i915/display/intel_psr.c +++ b/drivers/gpu/drm/i915/display/intel_psr.c @@ -1582,6 +1582,7 @@ static bool _psr_compute_config(struct intel_dp *intel_dp, if (entry_setup_frames >= 0) { intel_dp->psr.entry_setup_frames = entry_setup_frames; } else { + crtc_state->no_psr_reason = "PSR setup timing not met"; drm_dbg_kms(display->drm, "PSR condition failed: PSR setup timing not met\n"); return false; @@ -1813,6 +1814,7 @@ static void intel_psr_activate(struct intel_dp *intel_dp) hsw_activate_psr1(intel_dp); intel_dp->psr.active = true; + intel_dp->psr.no_psr_reason = NULL; } /* @@ -2925,6 +2927,9 @@ void intel_psr_pre_plane_update(struct intel_atomic_state *state, mutex_lock(&psr->lock); + if (!new_crtc_state->has_psr) + psr->no_psr_reason = new_crtc_state->no_psr_reason; + if (psr->enabled) { /* * Reasons to disable: @@ -2973,12 +2978,19 @@ void intel_psr_post_plane_update(struct intel_atomic_state *state, drm_WARN_ON(display->drm, psr->enabled && !crtc_state->active_planes); - keep_disabled |= psr->sink_not_reliable; - keep_disabled |= !crtc_state->active_planes; + if (psr->sink_not_reliable) + keep_disabled = true; + + if (!crtc_state->active_planes) { + psr->no_psr_reason = "All planes inactive"; + keep_disabled = true; + } /* Display WA #1136: skl, bxt */ - keep_disabled |= DISPLAY_VER(display) < 11 && - crtc_state->wm_level_disabled; + if (DISPLAY_VER(display) < 11 && crtc_state->wm_level_disabled) { + psr->no_psr_reason = "Workaround #1136 for skl, bxt"; + keep_disabled = true; + } if (!psr->enabled && !keep_disabled) intel_psr_enable_locked(intel_dp, crtc_state); @@ -4017,6 +4029,8 @@ static void intel_psr_print_mode(struct intel_dp *intel_dp, region_et = ""; seq_printf(m, "PSR mode: %s%s%s\n", mode, status, region_et); + if (psr->no_psr_reason) + seq_printf(m, " %s\n", psr->no_psr_reason); } static int intel_psr_status(struct seq_file *m, struct intel_dp *intel_dp) -- 2.45.2 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v11 1/1] drm/i915/display: Add no_psr_reason to PSR debugfs 2025-09-15 14:19 ` [PATCH v11 1/1] " Michał Grzelak @ 2025-09-30 4:38 ` Hogander, Jouni 2025-10-02 9:09 ` Michał Grzelak 0 siblings, 1 reply; 4+ messages in thread From: Hogander, Jouni @ 2025-09-30 4:38 UTC (permalink / raw) To: intel-xe@lists.freedesktop.org, Grzelak, Michal, intel-gfx@lists.freedesktop.org Cc: Brzezinka, Sebastian On Mon, 2025-09-15 at 16:19 +0200, Grzelak, Michal wrote: > There is no reason in debugfs why PSR has been disabled. Currently, > without this information, IGT tests cannot decide whether PSR has > been disabled on purpose or was it abnormal behavior. Because of it, > the status of the test cannot be decided correctly. > > Add no_psr_reason field into struct intel_psr. Add no_psr_reason > into struct intel_crtc_state to prevent staying out of sync when > _psr_compute_config is not using computed state. Write the reason, > e.g. PSR setup timing not met, into proper PSR debugfs file. Update > the reason from new_crtc_state in intel_psr_pre_plane_update. Extend > format of debugfs file to have reason when it is non-NULL. Clean the > reason when PSR is activated. > > Refactor intel_psr_post_plane_update to use no_psr_reason along > keep_disabled. > > Changelog: > v10->v11 > - update the reason from new_crtc_state instead of old_crtc_state > [Jouni] > > v9->v10 > - log reason into intel_crtc_state instead of intel_dp->psr [Jouni] > - remove clearing no_psr_reason in intel_psr_compute_config [Jouni] > - change update of no_psr_reason into more readable form [Jouni] > > v8->v9 > - add no_psr_reason into struct intel_crtc_state [Jouni] > - update the reason in intel_psr_pre_plane_update [Jouni] > - elaborate on motivation of the feature in commit message > [Sebastian] > - copy changelog to commit message [Sebastian] > > v7->v8 > - reset no_psr_reason at the begin of intel_psr_compute_config > [Jouni] > - restore keep_disabled [Jouni] > - drop setting "Sink not reliable" [Jouni] > - add WA number [Jouni] > - if non-NULL, write no_psr_reason after PSR mode [Jouni] > > v6->v7 > - rebase onto new drm-tip > > v5->v6 > - move setting no_psr_reason to intel_psr_post_plane_update [Jouni] > - remove setting no_psr_reason when disabling PSR is temporary > [Jouni] > > v4->v5 > - fix indentation errors from checkpatch > > v3->v4 > - change format of logging workaround #1136 > > v2->v3 > - change reason description to be more specific [Mika] > - remove BSpecs number & WA number from being written into > no_psr_reason > - replace spaces with tabs > > v1->v2 > - set other reasons than "PSR setup timing not met" > - clear no_psr_reason when activating PSR. > > Reviewed-by: Sebastian Brzezinka <sebastian.brzezinka@intel.com> > Signed-off-by: Michał Grzelak <michal.grzelak@intel.com> Hopefully you have time and interest to extend this. Anyways this one together with your igt patch is already solving one problem for us: Reviewed-by: Jouni Högander <jouni.hogander@intel.com> > --- > .../drm/i915/display/intel_display_types.h | 3 +++ > drivers/gpu/drm/i915/display/intel_psr.c | 22 +++++++++++++++-- > -- > 2 files changed, 21 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h > b/drivers/gpu/drm/i915/display/intel_display_types.h > index 358ab922d7a7..8e930ec197ab 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > @@ -1127,6 +1127,7 @@ struct intel_crtc_state { > u32 dc3co_exitline; > u16 su_y_granularity; > u8 active_non_psr_pipes; > + const char *no_psr_reason; > > /* > * Frequency the dpll for the port should run at. Differs > from the > @@ -1689,6 +1690,8 @@ struct intel_psr { > bool pkg_c_latency_used; > > u8 active_non_psr_pipes; > + > + const char *no_psr_reason; > }; > > struct intel_dp { > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c > b/drivers/gpu/drm/i915/display/intel_psr.c > index 01bf304c705f..a1faf8a8d184 100644 > --- a/drivers/gpu/drm/i915/display/intel_psr.c > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > @@ -1582,6 +1582,7 @@ static bool _psr_compute_config(struct intel_dp > *intel_dp, > if (entry_setup_frames >= 0) { > intel_dp->psr.entry_setup_frames = > entry_setup_frames; > } else { > + crtc_state->no_psr_reason = "PSR setup timing not > met"; > drm_dbg_kms(display->drm, > "PSR condition failed: PSR setup timing > not met\n"); > return false; > @@ -1813,6 +1814,7 @@ static void intel_psr_activate(struct intel_dp > *intel_dp) > hsw_activate_psr1(intel_dp); > > intel_dp->psr.active = true; > + intel_dp->psr.no_psr_reason = NULL; > } > > /* > @@ -2925,6 +2927,9 @@ void intel_psr_pre_plane_update(struct > intel_atomic_state *state, > > mutex_lock(&psr->lock); > > + if (!new_crtc_state->has_psr) > + psr->no_psr_reason = new_crtc_state- > >no_psr_reason; > + > if (psr->enabled) { > /* > * Reasons to disable: > @@ -2973,12 +2978,19 @@ void intel_psr_post_plane_update(struct > intel_atomic_state *state, > drm_WARN_ON(display->drm, > psr->enabled && !crtc_state- > >active_planes); > > - keep_disabled |= psr->sink_not_reliable; > - keep_disabled |= !crtc_state->active_planes; > + if (psr->sink_not_reliable) > + keep_disabled = true; > + > + if (!crtc_state->active_planes) { > + psr->no_psr_reason = "All planes inactive"; > + keep_disabled = true; > + } > > /* Display WA #1136: skl, bxt */ > - keep_disabled |= DISPLAY_VER(display) < 11 && > - crtc_state->wm_level_disabled; > + if (DISPLAY_VER(display) < 11 && crtc_state- > >wm_level_disabled) { > + psr->no_psr_reason = "Workaround #1136 for > skl, bxt"; > + keep_disabled = true; > + } > > if (!psr->enabled && !keep_disabled) > intel_psr_enable_locked(intel_dp, > crtc_state); > @@ -4017,6 +4029,8 @@ static void intel_psr_print_mode(struct > intel_dp *intel_dp, > region_et = ""; > > seq_printf(m, "PSR mode: %s%s%s\n", mode, status, > region_et); > + if (psr->no_psr_reason) > + seq_printf(m, " %s\n", psr->no_psr_reason); > } > > static int intel_psr_status(struct seq_file *m, struct intel_dp > *intel_dp) > -- > 2.45.2 > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v11 1/1] drm/i915/display: Add no_psr_reason to PSR debugfs 2025-09-30 4:38 ` Hogander, Jouni @ 2025-10-02 9:09 ` Michał Grzelak 0 siblings, 0 replies; 4+ messages in thread From: Michał Grzelak @ 2025-10-02 9:09 UTC (permalink / raw) To: Hogander, Jouni Cc: intel-xe@lists.freedesktop.org, Grzelak, Michal, intel-gfx@lists.freedesktop.org, Brzezinka, Sebastian [-- Attachment #1: Type: text/plain, Size: 3163 bytes --] On Tue, 30 Sep 2025, Hogander, Jouni wrote: > On Mon, 2025-09-15 at 16:19 +0200, Grzelak, Michal wrote: >> There is no reason in debugfs why PSR has been disabled. Currently, >> without this information, IGT tests cannot decide whether PSR has >> been disabled on purpose or was it abnormal behavior. Because of it, >> the status of the test cannot be decided correctly. >> >> Add no_psr_reason field into struct intel_psr. Add no_psr_reason >> into struct intel_crtc_state to prevent staying out of sync when >> _psr_compute_config is not using computed state. Write the reason, >> e.g. PSR setup timing not met, into proper PSR debugfs file. Update >> the reason from new_crtc_state in intel_psr_pre_plane_update. Extend >> format of debugfs file to have reason when it is non-NULL. Clean the >> reason when PSR is activated. >> >> Refactor intel_psr_post_plane_update to use no_psr_reason along >> keep_disabled. >> >> Changelog: >> v10->v11 >> - update the reason from new_crtc_state instead of old_crtc_state >> [Jouni] >> >> v9->v10 >> - log reason into intel_crtc_state instead of intel_dp->psr [Jouni] >> - remove clearing no_psr_reason in intel_psr_compute_config [Jouni] >> - change update of no_psr_reason into more readable form [Jouni] >> >> v8->v9 >> - add no_psr_reason into struct intel_crtc_state [Jouni] >> - update the reason in intel_psr_pre_plane_update [Jouni] >> - elaborate on motivation of the feature in commit message >> [Sebastian] >> - copy changelog to commit message [Sebastian] >> >> v7->v8 >> - reset no_psr_reason at the begin of intel_psr_compute_config >> [Jouni] >> - restore keep_disabled [Jouni] >> - drop setting "Sink not reliable" [Jouni] >> - add WA number [Jouni] >> - if non-NULL, write no_psr_reason after PSR mode [Jouni] >> >> v6->v7 >> - rebase onto new drm-tip >> >> v5->v6 >> - move setting no_psr_reason to intel_psr_post_plane_update [Jouni] >> - remove setting no_psr_reason when disabling PSR is temporary >> [Jouni] >> >> v4->v5 >> - fix indentation errors from checkpatch >> >> v3->v4 >> - change format of logging workaround #1136 >> >> v2->v3 >> - change reason description to be more specific [Mika] >> - remove BSpecs number & WA number from being written into >> no_psr_reason >> - replace spaces with tabs >> >> v1->v2 >> - set other reasons than "PSR setup timing not met" >> - clear no_psr_reason when activating PSR. >> >> Reviewed-by: Sebastian Brzezinka <sebastian.brzezinka@intel.com> >> Signed-off-by: Michał Grzelak <michal.grzelak@intel.com> > > Hopefully you have time and interest to extend this. Anyways this one > together with your igt patch is already solving one problem for us: > > Reviewed-by: Jouni Högander <jouni.hogander@intel.com> > Thanks Jouni for your review and all the comments throughout the series. Certainly there is more work to do, e.g. adding reason for no PSR2, or maybe even creating separate debugfs interface for no_psr_reason. For now, I will just stick to resending the patch with your R-B; this is also in order to retrigger CI run, because apparently since the IGT got merged, the Test-with: tag tripped Patchwork and no testing has been done. BR, Michał ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-10-02 9:09 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-09-15 14:19 [PATCH v11 0/1] drm/i915/display: Add no_psr_reason to PSR debugfs Michał Grzelak 2025-09-15 14:19 ` [PATCH v11 1/1] " Michał Grzelak 2025-09-30 4:38 ` Hogander, Jouni 2025-10-02 9:09 ` Michał Grzelak
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox