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

[Dibin Moolakadan Subrahmanian <dibin.moolakadan.subrahmanian@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 7004 bytes --]


On 16-12-2025 14:00, Hogander, Jouni wrote:
> On Tue, 2025-12-16 at 13:54 +0530, Dibin Moolakadan Subrahmanian wrote:
> >
> >
> > On 12-12-2025 12:41, Hogander, Jouni wrote:
> >
> >> 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?
> > For both LOBF and PSR dc3co will be enabled from intel_atomic_commit_tail() it self.
> > This patch selects DC6 after 6 idle frames for PSR.
>
> But how you are planning to do the same for LOBF? I think they should have
> common control.

Planning not to enable DC6 for LOBF, only DC3CO will be enabled in LOBF.

> BR,
> Jouni Högander
>
> >
> >> 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);
> >>>   }
> >>>   
>

[-- Attachment #2: Type: text/html, Size: 9119 bytes --]