Re: [PATCH v2 2/2] drm/i915/psr: Do not disable Panel Replay if PSR2 is disabled

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

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 <jouni.hogander@intel.com>
> ---
>  .../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)");
>  
>  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) ||

I believe we should probably define the bits

#define PSR_PARAM_DISABLE_PSR2		BIT(0)
#define PSR_PARAM_DISABLE_PANEL_REPLAY	BIT(1)

likely here in this .c file itself, not needed to be along with the param
but up to you, if you believe it makes more sense and gets clear there...

>  	    (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
>