Re: [PATCH 9/9] drm/i915/display: Add DC3CO disable handling for psr2

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

On Tue, 2025-12-09 at 17:03 +0530, Dibin Moolakadan Subrahmanian wrote:
> dc6 should be enabled instead of dc3co after  6 idle frames
> while in psr2.(re enable part of tgl dc3co handling)

This is for PSR and pretty much following existing (disabled) TGL DC3CO
control. What is your idea how selection between DC6 and DC3CO will be
done in case of LOBF?

BR,

Jouni Högander

> 
> Signed-off-by: Dibin Moolakadan Subrahmanian
> <dibin.moolakadan.subrahmanian@intel.com>
> ---
>  .../drm/i915/display/intel_display_types.h    |  1 +
>  drivers/gpu/drm/i915/display/intel_psr.c      | 78
> ++++++++++++++++++-
>  2 files changed, 78 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 27f69df7ee9c..6ff53cd58052 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1759,6 +1759,7 @@ struct intel_psr {
>  	bool panel_replay_enabled;
>  	u32 dc3co_exitline;
>  	u32 dc3co_exit_delay;
> +	struct delayed_work dc3co_work;
>  	u8 entry_setup_frames;
>  
>  	u8 io_wake_lines;
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> b/drivers/gpu/drm/i915/display/intel_psr.c
> index 18bf45455ea2..4be709d1d324 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -1157,6 +1157,78 @@ static void psr2_program_idle_frames(struct
> intel_dp *intel_dp,
>  		     EDP_PSR2_IDLE_FRAMES(idle_frames));
>  }
>  
> +static void psr2_dc3co_disable(struct intel_dp *intel_dp)
> +{
> +	struct intel_display *display = to_intel_display(intel_dp);
> +	struct i915_power_domains *power_domains = &display-
> >power.domains;
> +
> +	if ((power_domains->allowed_dc_mask &
> DC_STATE_EN_UPTO_DC3CO) != DC_STATE_EN_UPTO_DC3CO)
> +		return;
> +
> +	intel_display_power_set_target_dc_state(display,
> DC_STATE_EN_UPTO_DC6);
> +	/* Todo restore PSR2 idle frames , ALPM control*/
> +}
> +
> +static void psr2_dc3co_disable_on_exit(struct intel_dp *intel_dp)
> +{
> +	struct intel_display *display = to_intel_display(intel_dp);
> +	struct i915_power_domains *power_domains = &display-
> >power.domains;
> +
> +	if ((power_domains->allowed_dc_mask &
> DC_STATE_EN_UPTO_DC3CO) != DC_STATE_EN_UPTO_DC3CO)
> +		return;
> +
> +	cancel_delayed_work(&intel_dp->psr.dc3co_work);
> +	intel_dc3co_source_unset(display, DC3CO_SOURCE_PSR2);
> +}
> +
> +static void psr2_dc3co_disable_work(struct work_struct *work)
> +{
> +	struct intel_dp *intel_dp =
> +		container_of(work, typeof(*intel_dp),
> psr.dc3co_work.work);
> +
> +	mutex_lock(&intel_dp->psr.lock);
> +	/* If delayed work is pending, it is not idle */
> +	if (delayed_work_pending(&intel_dp->psr.dc3co_work))
> +		goto unlock;
> +	/* enable DC6 after idle frames*/
> +	psr2_dc3co_disable(intel_dp);
> +
> +unlock:
> +	mutex_unlock(&intel_dp->psr.lock);
> +}
> +
> +/*
> + * When we will be completely rely on PSR2 S/W tracking in future,
> + * intel_psr_flush() will invalidate and flush the PSR for
> ORIGIN_FLIP
> + * event also therefore psr2_dc3co_flush_locked() require to be
> changed
> + * accordingly in future.
> + */
> +
> +static void
> +psr2_dc3co_flush_locked(struct intel_dp *intel_dp, unsigned int
> frontbuffer_bits,
> +			enum fb_op_origin origin)
> +{
> +	struct intel_display *display = to_intel_display(intel_dp);
> +	struct i915_power_domains *power_domains = &display-
> >power.domains;
> +
> +	if (!(power_domains->allowed_dc_mask &
> DC_STATE_EN_UPTO_DC3CO))
> +		return;
> +
> +	if (!intel_dp->psr.sel_update_enabled ||
> +	    !intel_dp->psr.active)
> +		return;
> +	/*
> +	 * At every frontbuffer flush flip event modified delay of
> delayed work,
> +	 * when delayed work schedules that means display has been
> idle.
> +	 */
> +	if (!(frontbuffer_bits &
> +	    INTEL_FRONTBUFFER_ALL_MASK(intel_dp->psr.pipe)))
> +		return;
> +
> +	mod_delayed_work(display->wq.unordered, &intel_dp-
> >psr.dc3co_work,
> +			 intel_dp->psr.dc3co_exit_delay);
> +}
> +
>  static bool intel_psr2_sel_fetch_config_valid(struct intel_dp
> *intel_dp,
>  					      struct
> intel_crtc_state *crtc_state)
>  {
> @@ -2117,7 +2189,7 @@ static void intel_psr_exit(struct intel_dp
> *intel_dp)
>  		intel_de_rmw(display, TRANS_DP2_CTL(intel_dp-
> >psr.transcoder),
>  			     TRANS_DP2_PANEL_REPLAY_ENABLE, 0);
>  	} else if (intel_dp->psr.sel_update_enabled) {
> -
> +		psr2_dc3co_disable_on_exit(intel_dp);
>  		val = intel_de_rmw(display,
>  				   EDP_PSR2_CTL(display,
> cpu_transcoder),
>  				   EDP_PSR2_ENABLE, 0);
> @@ -2259,6 +2331,7 @@ void intel_psr_disable(struct intel_dp
> *intel_dp,
>  
>  	mutex_unlock(&intel_dp->psr.lock);
>  	cancel_work_sync(&intel_dp->psr.work);
> +	cancel_delayed_work_sync(&intel_dp->psr.dc3co_work);
>  }
>  
>  /**
> @@ -2289,6 +2362,7 @@ void intel_psr_pause(struct intel_dp *intel_dp)
>  	mutex_unlock(&psr->lock);
>  
>  	cancel_work_sync(&psr->work);
> +	cancel_delayed_work_sync(&psr->dc3co_work);
>  }
>  
>  /**
> @@ -3475,6 +3549,7 @@ void intel_psr_flush(struct intel_display
> *display,
>  		if (origin == ORIGIN_FLIP ||
>  		    (origin == ORIGIN_CURSOR_UPDATE &&
>  		     !intel_dp->psr.psr2_sel_fetch_enabled)) {
> +			psr2_dc3co_flush_locked(intel_dp,
> frontbuffer_bits, origin);
>  			goto unlock;
>  		}
>  
> @@ -3533,6 +3608,7 @@ void intel_psr_init(struct intel_dp *intel_dp)
>  		intel_dp->psr.link_standby = connector-
> >panel.vbt.psr.full_link;
>  
>  	INIT_WORK(&intel_dp->psr.work, intel_psr_work);
> +	INIT_DELAYED_WORK(&intel_dp->psr.dc3co_work,
> psr2_dc3co_disable_work);
>  	mutex_init(&intel_dp->psr.lock);
>  }
>