Hi Sebastian,
From: Brzezinka, Sebastian <sebastian.brzezi...@intel.com>
Sent: Thursday, August 28, 2025 4:12 PM
>Hi Michał
>On Thu Aug 28, 2025 at 10:49 AM UTC, Michał Grzelak wrote:
>> There is no reason in debugfs why PSR has been disabled. Add
>It might be useful to explain the motivation behind this feature.
Do you think it will be sufficient to put the first sentence this way then?:
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.
>> no_psr_reason field into struct intel_psr. Write the reason, e.g. PSR
>> setup timing not met, into proper PSR debugfs file.
>> Extend format of debugfs file to have reason when non-NULL.
>> Ensure no_psr_reason is up-to-date or NULL by resetting it at the
>> beginning of intel_psr_compute_config. Clean it when PSR is activated.
>>
>> Refactor intel_psr_post_plane_update to use no_psr_reason along
>> keep_disabled.
>It appears to be version 8. I think it would be helpful to include a
>changelog.
Ok, will copy changelog from cover letter and add it to the commit message
in next version of the patch.
>>
>> Signed-off-by: Michał Grzelak <michal.grze...@intel.com>
>> ---
>> .../drm/i915/display/intel_display_types.h | 2 ++
>> drivers/gpu/drm/i915/display/intel_psr.c | 21 +++++++++++++++----
>> 2 files changed, 19 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 fd9d2527889b..0f8332ce1aa0 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
>> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
>> @@ -1687,6 +1687,8 @@ struct intel_psr {
>> bool pkg_c_latency_used;
>>
>> u8 active_non_psr_pipes;
>> +
>> + const char *no_psr_reason;
>Have you considered using an enum or another type instead of a char pointer?
I was reflecting other features in the driver which use it the same way, e.g.
const char *no_fbc_reason from drivers/gpu/drm/i915/display/intel_fbc.c.
Besides of it, I used char pointer as it doesn't require type conversion when
writing to debugfs file. Do you think I should change it to enum instead?
Best regards,
Michał
