[PATCH v5 1/1] drm/i915/display: Add no_psr_reason to PSR debugfs

[Grzelak, Michal]

Hi Krzysztof,

>From: Karas, Krzysztof <krzysztof.karas at intel.com> 
>Sent: Thursday, July 17, 2025 1:09 PM
>
>Hi Michał,
>
>[...]
>> @@ -2937,13 +2939,21 @@ void intel_psr_pre_plane_update(struct intel_atomic_state *state,
>>  			 * - Region Early Transport changing
>>  			 * - Display WA #1136: skl, bxt
>>  			 */
>> -			if (intel_crtc_needs_modeset(new_crtc_state) ||
>> -			    !new_crtc_state->has_psr ||
>> -			    !new_crtc_state->active_planes ||
>> -			    new_crtc_state->has_sel_update != psr->sel_update_enabled ||
>> -			    new_crtc_state->enable_psr2_su_region_et != psr->su_region_et_enabled ||
>> -			    new_crtc_state->has_panel_replay != psr->panel_replay_enabled ||
>> -			    (DISPLAY_VER(display) < 11 && new_crtc_state->wm_level_disabled))
>> +			if (intel_crtc_needs_modeset(new_crtc_state))
>> +				psr->no_psr_reason = "CRTC needs modeset";
>> +			if (!new_crtc_state->has_psr)
>> +				psr->no_psr_reason = "PSR disabled";
>> +			if (!new_crtc_state->active_planes)
>> +				psr->no_psr_reason = "All planes inactive";
>> +			if (new_crtc_state->has_sel_update != psr->sel_update_enabled)
>> +				psr->no_psr_reason = "Changing between PSR versions";
>> +			if (new_crtc_state->enable_psr2_su_region_et != psr->su_region_et_enabled)
>> +				psr->no_psr_reason = "Changing Region Early Transport";
>> +			if (new_crtc_state->has_panel_replay != psr->panel_replay_enabled)
>> +				psr->no_psr_reason = "Changing Panel Replay mode";
>> +			if (DISPLAY_VER(display) < 11 && new_crtc_state->wm_level_disabled)
>> +				psr->no_psr_reason = "Wa_1136";
>> +			if (psr->no_psr_reason)
>>  				intel_psr_disable_locked(intel_dp);
>>  			else if (new_crtc_state->wm_level_disabled)
>>  				/* Wa_14015648006 */
>Is it possible to have multiple reasons for disabling psr?
>The way no_psr_reason is set above causes only the last reason to be logged into that field.

I think it is possible to have multiple reasons, but I don't think it is significant to store each
observed reason at one time though. The intention behind no_psr_reason is similar to
no_fbc_reason in intel_fbc.c, so to prevent adding a list of happened/not-happened flags
of reasons to `struct intel_psr`, and encompass all of them in one string.

Currently, when PSR is disabled, there is no reason logged, so I think it would be beneficial
to store & log any reason.

Best regards,
Michał