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

[Jani Nikula <jani.nikula@linux.intel.com>]

On Tue, 09 Dec 2025, Dibin Moolakadan Subrahmanian <dibin.moolakadan.subrahmanian@intel.com> wrote:
> dc6 should be enabled instead of dc3co after  6 idle frames
> while in psr2.(re enable part of tgl dc3co handling)

Please write proper commit messages. I don't understand what this patch
is supposed to do based on this.

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

There's currently one place in intel_psr.c that checks
power_domains->allowed_dc_mask, and I think even that is too much.

display->power belongs to intel_display_power*.c, and nobody else.

I think you probably need a helper function to ask for this stuff from
power modules.

> +
> +	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*/

	/* 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);
>  }

-- 
Jani Nikula, Intel