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

Reply via email to