On Tue, 2025-07-08 at 16:18 -0400, Rodrigo Vivi wrote:
> On Tue, Jul 08, 2025 at 05:39:08AM +0000, Hogander, Jouni wrote:
> > On Mon, 2025-07-07 at 11:01 -0400, Rodrigo Vivi wrote:
> > > On Mon, Jul 07, 2025 at 01:47:33PM +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
> > > >
> > > > Signed-off-by: Jouni Högander <jouni.hogan...@intel.com>
> > > > ---
> > > > .../drm/i915/display/intel_display_params.c | 3 +--
> > > > drivers/gpu/drm/i915/display/intel_psr.c | 20
> > > > +++++++++++++--
> > > > ----
> > > > 2 files changed, 15 insertions(+), 8 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..1ba17ea40bba 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_display_params.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_display_params.c
> > > > @@ -116,8 +116,7 @@
> > > > 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) "
> > > > + "Enable PSR (0=disabled, 1=disable PSR2, 2=disable
> > > > Panel
> > > > Replay)"
> > >
> > > What about a bit mask?
> > >
> > > PSR1 = BIT0
> > > PSR2 = BIT1 (PSR2 infers PSR1 enabled)
> > > PANEL_REPLAY = BIT2 (also infers PSR1(and 2?) enabled)
> > >
> > > (Peraps even bit3 for early transport?)
> > >
> > > This is backwards compatible because
> > >
> > > 0 = disabled,
> > > 1 = up to psr1,
> > > 2 = up to psr2, (no panel replay)
> > > 3 = up to psr2, (same as 2)
> > > 4 = panel replay on
> > > 5 = same as 5
> >
> > Original problem behind my patch is enable_psr=1 having impact on
> > Panel
> > Replay. First I thought I Fix this by just ignoring enable_psr on
> > Panel
> > Replay. That would mean adding enable_panel_replay parameter as
> > well. I
> > didn't want to do that. We have user wanting to have PSR2
> > specifically
> > disabled but still use Panel Replay.
> > ->
> > I wanted to have enable_psr legacy values working as before without
> > having impact on Panel Replay. To have this I choose this meaning
> > to
> > values (bits):
> >
> > 0 = disable PSR/Panel Replay completely
> > 1 (BIT0) = disable PSR2 (allow PSR1/Panel Replay) (up to psr1)
> > 2 (BIT1) = disable Panel Replay (allow PSR1/PSR2) (up to psr2, (no
> > panel replay))
> >
> > I left Early Transport out here as disabling it means disabling
> > Panel
> > Replay as well. I also didn't want to make this parameter too
> > extensive/complex as we have separate debug interface to bisect PSR
> > features (/sys/kernel/debug/dri/*/i915_edp_psr_debug). Using this
> > disable bit approach is also easy to extent in future as needed.
> >
> > Your idea is following better meaning of "enable_psr".
>
> Well, but your idea captures better the issue and the backward
> compatibility. Let's go with your definition, but ensure to document
> the parameter like you described here above?
Just sent new version of my set. Please check.
BR,
Jouni Högander
>
> >
> > BR,
> >
> > Jouni Högander
> >
> > >
> > > > "Default: -1 (use per-chip default)");
> > > >
> > > > intel_display_param_named(psr_safest_params, bool, 0400,
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > > > b/drivers/gpu/drm/i915/display/intel_psr.c
> > > > index a2b5688f0c82..3215a11baa66 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > > > @@ -254,13 +254,15 @@ 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;
> > > > +}
> > > > +
> > > > +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 +271,7 @@ 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 == 2 ||
> > > > (intel_dp->psr.debug &
> > > > I915_PSR_DEBUG_PANEL_REPLAY_DISABLE))
> > > > return false;
> > > > return true;
> > > > @@ -1415,6 +1417,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 +1525,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 +1672,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
> > > >
> >
