On Wed, 2026-05-20 at 14:42 +0200, Michał Grzelak wrote:
> On Wed, 20 May 2026, Jouni Högander wrote:
> > We are observing following warnings:
> >
> > *ERROR* power well DC_off state mismatch (refcount 0/enabled 1)
> >
> > gen9_dc_off_power_well_enabled is considering target state
> > DC_STATE_DISABLE
> > as DC_OFF power well being enabled. Fix this by using wakeref for
> > the
> > purpose.
> >
> > To achieve this we need to modify notification code as well.
> > Currently it
> > is possible that PSR gets notified vblank enable/disable twice on
> > same
> > status. This is currently not a problem as it is just triggering
> > call to
> > intel_display_power_set_target_dc_state with same target state as a
> > parameter. When using wakeref this becomes a problem due to
> > reference
> > counting. Fix this storing vbank status on last notification and
> > use that
> > to ensure there are no more than one notification with same vblank
> > status.
> >
> > v2: ensure there is no subsequent notifications with same status
> >
> > Fixes: aa451abcffb5 ("drm/i915/display: Prevent DC6 while vblank is
> > enabled for Panel Replay")
> > Cc: <[email protected]> # v6.13+
> > Signed-off-by: Jouni Högander <[email protected]>
> > ---
> > .../gpu/drm/i915/display/intel_display_core.h | 1 +
> > .../gpu/drm/i915/display/intel_display_irq.c | 8 +++++--
> > .../drm/i915/display/intel_display_types.h | 2 ++
> > drivers/gpu/drm/i915/display/intel_psr.c | 24 +++++++---------
> > ---
> > 4 files changed, 18 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_core.h
> > b/drivers/gpu/drm/i915/display/intel_display_core.h
> > index 3dc5ac75a98b..64c1365fb366 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_core.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_core.h
> > @@ -494,6 +494,7 @@ struct intel_display {
> > u8 vblank_enabled;
> >
> > int vblank_enable_count;
> > + bool last_vblank_status_notified;
>
> couple of the fields in the irq sub-struct are prepended with vblank,
> thus
> wondering if we can do it here as well. Maybe
> vblank_last_status_notified? or vblank_status_last_notified?
>
> Anyways, for the v2:
>
> Reviewed-by: Michał Grzelak <[email protected]>
Thank you Michal for your review. With suggested change pushed now to
drm-intel-next.
BR,
Jouni Högander
>
> BR,
> Michał
>
> >
> > struct work_struct vblank_notify_work;
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_irq.c
> > b/drivers/gpu/drm/i915/display/intel_display_irq.c
> > index 899a38c0a7b7..57f37f9b83a5 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_irq.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display_irq.c
> > @@ -1786,8 +1786,12 @@ static void
> > intel_display_vblank_notify_work(struct work_struct *work)
> > struct intel_display *display =
> > container_of(work, typeof(*display),
> > irq.vblank_notify_work);
> > int vblank_enable_count = READ_ONCE(display-
> > >irq.vblank_enable_count);
> > + bool vblank_status = !!vblank_enable_count;
> >
> > - intel_psr_notify_vblank_enable_disable(display,
> > vblank_enable_count);
> > + if (display->irq.last_vblank_status_notified !=
> > vblank_status) {
> > + intel_psr_notify_vblank_enable_disable(display,
> > vblank_status);
> > + display->irq.last_vblank_status_notified =
> > vblank_status;
> > + }
> > }
> >
> > int bdw_enable_vblank(struct drm_crtc *_crtc)
> > @@ -1800,10 +1804,10 @@ int bdw_enable_vblank(struct drm_crtc
> > *_crtc)
> > if (gen11_dsi_configure_te(crtc, true))
> > return 0;
> >
> > + spin_lock_irqsave(&display->irq.lock, irqflags);
> > if (crtc->vblank_psr_notify && display-
> > >irq.vblank_enable_count++ == 0)
> > schedule_work(&display->irq.vblank_notify_work);
> >
> > - spin_lock_irqsave(&display->irq.lock, irqflags);
> > bdw_enable_pipe_irq(display, pipe, GEN8_PIPE_VBLANK);
> > spin_unlock_irqrestore(&display->irq.lock, irqflags);
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> > b/drivers/gpu/drm/i915/display/intel_display_types.h
> > index f44be5c689ae..b8ccd635c575 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > @@ -1790,6 +1790,8 @@ struct intel_psr {
> > u8 active_non_psr_pipes;
> >
> > const char *no_psr_reason;
> > +
> > + struct ref_tracker *vblank_wakeref;
> > };
> >
> > struct intel_dp {
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > b/drivers/gpu/drm/i915/display/intel_psr.c
> > index 70108e0a4c0c..19cfb23fe9f8 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -4180,14 +4180,20 @@ void
> > intel_psr_notify_vblank_enable_disable(struct intel_display
> > *display,
> > bool enable)
> > {
> > struct intel_encoder *encoder;
> > - bool block_dc_states = false;
> >
> > for_each_intel_encoder_with_psr(display->drm, encoder) {
> > struct intel_dp *intel_dp =
> > enc_to_intel_dp(encoder);
> >
> > mutex_lock(&intel_dp->psr.lock);
> > - if (CAN_PANEL_REPLAY(intel_dp))
> > - block_dc_states = true;
> > + if (CAN_PANEL_REPLAY(intel_dp)) {
> > + if (enable)
> > + intel_dp->psr.vblank_wakeref =
> > + intel_display_power_get(di
> > splay,
> > + PO
> > WER_DOMAIN_DC_OFF);
> > + else
> > + intel_display_power_put(display,
> > POWER_DOMAIN_DC_OFF,
> > + intel_dp-
> > >psr.vblank_wakeref);
> > + }
> >
> > if (intel_dp->psr.enabled && !intel_dp-
> > >psr.panel_replay_enabled &&
> > intel_dp->psr.pkg_c_latency_used)
> > @@ -4195,18 +4201,6 @@ void
> > intel_psr_notify_vblank_enable_disable(struct intel_display
> > *display,
> >
> > mutex_unlock(&intel_dp->psr.lock);
> > }
> > -
> > - /*
> > - * NOTE: intel_display_power_set_target_dc_state is used
> > - * only by PSR code for DC3CO handling. DC3CO target
> > - * state is currently disabled in * PSR code. If DC3CO
> > - * is taken into use we need take that into account here
> > - * as well.
> > - */
> > - if (block_dc_states)
> > - intel_display_power_set_target_dc_state(display,
> > enable ?
> > -
> > DC_STATE_DISABLE :
> > -
> > DC_STATE_EN_UPTO_DC6);
> > }
> >
> > static void
> > --
> > 2.43.0
> >