On Tue, 09 Dec 2025, Dibin Moolakadan Subrahmanian <[email protected]> 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 > <[email protected]> > --- > .../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
