Re: [PATCH 4/4] drm/i915/psr: Allow changing Panel Replay mode without full modeset

["Hogander, Jouni" <jouni.hogander@intel.com>]

On Mon, 2025-01-13 at 11:22 +0000, Kahola, Mika wrote:
> > -----Original Message-----
> > From: Intel-xe <intel-xe-bounces@lists.freedesktop.org> On Behalf
> > Of Jouni
> > Högander
> > Sent: Thursday, 9 January 2025 12.36
> > To: intel-gfx@lists.freedesktop.org; intel-xe@lists.freedesktop.org
> > Cc: Hogander, Jouni <jouni.hogander@intel.com>
> > Subject: [PATCH 4/4] drm/i915/psr: Allow changing Panel Replay mode
> > without
> > full modeset
> > 
> > Currently we are forcing full modeset if Panel Replay mode is
> > changed. This is not
> > necessary as long as we are not changing sink PANEL REPLAY ENABLE
> > bit in
> > PANEL REPLAY ENABLE AND CONFIGURATION 1 register. This can be
> > achieved by
> > entering Panel Replay inactive mode (Live Frame mode) when Panel
> > Replay is
> > disabled and keep PANEL REPLAY ENABLE bit in PANEL REPLAY ENABLE
> > AND
> > CONFIGURATION 1 enabled always if panel is just supporting Panel
> > Replay.
> > 
> 
> Reviewed-by: Mika Kahola <mika.kahola@intel.com>

Thank you Mika for checking my patches. These are now pushed to drm-
intel-next.

BR,

Jouni Högander

> 
> > Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_display.c | 14 -----------
> >  drivers/gpu/drm/i915/display/intel_psr.c     | 25 +++++++++++++---
> > ----
> >  2 files changed, 17 insertions(+), 22 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > b/drivers/gpu/drm/i915/display/intel_display.c
> > index 21319f753a34..35499eb296b3 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -5638,20 +5638,6 @@ intel_pipe_config_compare(const struct
> > intel_crtc_state *current_config,
> >  		PIPE_CONF_CHECK_CSC(output_csc);
> >  	}
> > 
> > -	/*
> > -	 * Panel replay has to be enabled before link training.
> > PSR doesn't have
> > -	 * this requirement -> check these only if using panel
> > replay
> > -	 */
> > -	if (current_config->active_planes &&
> > -	    (current_config->has_panel_replay ||
> > -	     pipe_config->has_panel_replay)) {
> > -		PIPE_CONF_CHECK_BOOL(has_psr);
> > -		PIPE_CONF_CHECK_BOOL(has_sel_update);
> > -		PIPE_CONF_CHECK_BOOL(enable_psr2_sel_fetch);
> > -		PIPE_CONF_CHECK_BOOL(enable_psr2_su_region_et);
> > -		PIPE_CONF_CHECK_BOOL(has_panel_replay);
> > -	}
> > -
> >  	PIPE_CONF_CHECK_BOOL(double_wide);
> > 
> >  	if (dev_priv->display.dpll.mgr)
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > b/drivers/gpu/drm/i915/display/intel_psr.c
> > index f969abc56256..3494620d1c44 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -1998,18 +1998,25 @@ static void intel_psr_enable_locked(struct
> > intel_dp
> > *intel_dp,
> >  	if (!psr_interrupt_error_check(intel_dp))
> >  		return;
> > 
> > -	if (intel_dp->psr.panel_replay_enabled) {
> > +	if (intel_dp->psr.panel_replay_enabled)
> >  		drm_dbg_kms(display->drm, "Enabling Panel
> > Replay\n");
> > -	} else {
> > +	else
> >  		drm_dbg_kms(display->drm, "Enabling PSR%s\n",
> >  			    intel_dp->psr.sel_update_enabled ? "2"
> > : "1");
> > 
> > -		/*
> > -		 * Panel replay has to be enabled before link
> > training: doing it
> > -		 * only for PSR here.
> > -		 */
> > -		intel_psr_enable_sink(intel_dp, crtc_state);
> > -	}
> > +	/*
> > +	 * Enabling here only for PSR. Panel Replay enable bit is
> > already
> > +	 * written at this point. See
> > +	 * intel_psr_panel_replay_enable_sink. Modifiers/options:
> > +	 *  - Selective Update
> > +	 *  - Region Early Transport
> > +	 *  - Selective Update Region Scanline Capture
> > +	 *  - VSC_SDP_CRC
> > +	 *  - HPD on different Errors
> > +	 *  - CRC verification
> > +	 * are written for PSR and Panel Replay here.
> > +	 */
> > +	intel_psr_enable_sink(intel_dp, crtc_state);
> > 
> >  	if (intel_dp_is_edp(intel_dp))
> >  		intel_snps_phy_update_psr_power_state(&dig_port-
> > >base,
> > true); @@ -2815,6 +2822,8 @@ void intel_psr_pre_plane_update(struct
> > intel_atomic_state *state,
> >  		needs_to_disable |= new_crtc_state->has_sel_update
> > != psr-
> > > sel_update_enabled;
> >  		needs_to_disable |= new_crtc_state-
> > > enable_psr2_su_region_et !=
> >  			psr->su_region_et_enabled;
> > +		needs_to_disable |= new_crtc_state-
> > >has_panel_replay !=
> > +			psr->panel_replay_enabled;
> >  		needs_to_disable |= DISPLAY_VER(i915) < 11 &&
> >  			new_crtc_state->wm_level_disabled;
> > 
> > --
> > 2.34.1
>