On Wed, Jul 09, 2025 at 10:57:58AM +0300, Jouni Högander wrote:
> Currently disabling PSR2 via enable_psr module parameter causes Panel
> Replay being disabled as well. This patch changes this by still allowing
> Panel Replay even if PSR2 is disabled.
> 
> After this patch enable_psr module parameter values are:
> 
> -1 = PSR1 : yes, PSR2 = yes, Panel Replay : yes
>  0 = PSR1 : no,  PSR2 = no,  Panel Replay : no
>  1 = PSR1 : yes, PSR2 = no,  Panel Replay : yes
>  2 = PSR1 : yes, PSR2 = yes, Panel Replay : no
>  3 = PSR1 : yes, PSR2 = no,  Panel Replay : no
> 
> I.e. values different than -1 and 0 are handled as bitmasks where BIT0
> disables PSR2 and BIT1 disables Panel Replay.
> 
> v2:
>   - make it more clear that enable_psr is bitmask for disabling different
>     PSR modes
> 
> Signed-off-by: Jouni Högander <[email protected]>
> ---
>  .../drm/i915/display/intel_display_params.c   |  6 ++---
>  drivers/gpu/drm/i915/display/intel_psr.c      | 22 ++++++++++++++-----
>  2 files changed, 19 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_params.c 
> b/drivers/gpu/drm/i915/display/intel_display_params.c
> index 75316247ee8a..195af19ece5f 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_params.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_params.c
> @@ -116,9 +116,9 @@ intel_display_param_named_unsafe(enable_fbc, int, 0400,
>       "(default: -1 (use per-chip default))");
>  
>  intel_display_param_named_unsafe(enable_psr, int, 0400,
> -     "Enable PSR "
> -     "(0=disabled, 1=enable up to PSR1, 2=enable up to PSR2) "
> -     "Default: -1 (use per-chip default)");
> +     "Enable PSR (0=disabled, 1=disable PSR2 (BIT0), 2=disable Panel Replay 
> (BIT1))."
> +     "Values different from 0 and -1 are handled as bitmask to disable 
> different PSR modes."
> +     "E.g. value 3 disables both PSR2 and Panel Replay. Default: -1 (use 
> per-chip default)");

This thing is very unintuitive. Why don't we just get replace it
with a new disable_psr modparam that is clearly just a bitmask of
what to disable?

>  
>  intel_display_param_named(psr_safest_params, bool, 0400,
>       "Replace PSR VBT parameters by the safest and not optimal ones. This "
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c 
> b/drivers/gpu/drm/i915/display/intel_psr.c
> index a2b5688f0c82..959b868672d0 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -254,13 +254,16 @@ static bool psr2_global_enabled(struct intel_dp 
> *intel_dp)
>  {
>       struct intel_display *display = to_intel_display(intel_dp);
>  
> +     return display->params.enable_psr == -1 ||
> +             !(display->params.enable_psr & 0x1);
> +}
> +
> +static bool sel_update_global_enabled(struct intel_dp *intel_dp)
> +{
>       switch (intel_dp->psr.debug & I915_PSR_DEBUG_MODE_MASK) {
> -     case I915_PSR_DEBUG_DISABLE:
>       case I915_PSR_DEBUG_FORCE_PSR1:
>               return false;
>       default:
> -             if (display->params.enable_psr == 1)
> -                     return false;
>               return true;
>       }
>  }
> @@ -269,7 +272,8 @@ static bool panel_replay_global_enabled(struct intel_dp 
> *intel_dp)
>  {
>       struct intel_display *display = to_intel_display(intel_dp);
>  
> -     if ((display->params.enable_psr != -1) ||
> +     if ((display->params.enable_psr != -1 &&
> +          display->params.enable_psr & 0x2) ||
>           (intel_dp->psr.debug & I915_PSR_DEBUG_PANEL_REPLAY_DISABLE))
>               return false;
>       return true;
> @@ -1415,6 +1419,12 @@ static bool intel_psr2_config_valid(struct intel_dp 
> *intel_dp,
>       if (!intel_dp->psr.sink_psr2_support)
>               return false;
>  
> +     if (!psr2_global_enabled(intel_dp)) {
> +             drm_dbg_kms(display->drm,
> +                         "PSR2 disabled by flag\n");
> +             return false;
> +     }
> +
>       /* JSL and EHL only supports eDP 1.3 */
>       if (display->platform.jasperlake || display->platform.elkhartlake) {
>               drm_dbg_kms(display->drm, "PSR2 not supported by phy\n");
> @@ -1517,7 +1527,7 @@ static bool intel_sel_update_config_valid(struct 
> intel_dp *intel_dp,
>               goto unsupported;
>       }
>  
> -     if (!psr2_global_enabled(intel_dp)) {
> +     if (!sel_update_global_enabled(intel_dp)) {
>               drm_dbg_kms(display->drm,
>                           "Selective update disabled by flag\n");
>               goto unsupported;
> @@ -1664,7 +1674,7 @@ void intel_psr_compute_config(struct intel_dp *intel_dp,
>       u8 active_pipes = 0;
>  
>       if (!psr_global_enabled(intel_dp)) {
> -             drm_dbg_kms(display->drm, "PSR disabled by flag\n");
> +             drm_dbg_kms(display->drm, "PSR/Panel Replay disabled by 
> flag\n");
>               return;
>       }
>  
> -- 
> 2.43.0

-- 
Ville Syrjälä
Intel

Reply via email to