OK, confirmed. That patch fixes the issue

Thanks!



On Mon, Aug 31, 2026 at 7:52 AM Eduardo Díaz <[email protected]> wrote:

> I filed a report here:
> https://gitlab.freedesktop.org/drm/xe/kernel/-/work_items/9074 debug logs
> weren't enabled though.
>
> I don't believe I've tried 1786d2688781 ("drm/i915/cdclk: Avoid spurious
> cdclk
> sanitization on PTL+"), will do so later today.
>
> On Mon, Aug 31, 2026 at 12:39 AM Jani Nikula <[email protected]>
> wrote:
>
>> On Sun, 30 Aug 2026, Eduardo Diaz <[email protected]> wrote:
>> > On Panther Lake (xe3lpd) laptops, cold boot reliably corrupts the
>> > internal eDP panel: pipe A gets a "Selective fetch area calculation
>> > failed in pipe A" warning immediately followed by a CPU pipe A FIFO
>> > underrun, and the panel stays corrupted for the rest of the session.
>> > A subsequent suspend/resume cycle (or any other full re-modeset)
>> > "fixes" it, which pointed at cdclk/voltage-level tracking rather than
>> > a genuine hardware race.
>> >
>> > intel_modeset_readout_hw_state() runs once at driver probe (and again
>> > on resume) to figure out what firmware/GOP left the display in. For
>> > each already-active pipe it calls intel_cdclk_update_hw_state(),
>> > which seeds cdclk_state->min_cdclk[]/min_voltage_level[] directly
>> > from the freshly read-out crtc_state. That treats "firmware left this
>> > pipe active with mode X" as proof that this driver's own cdclk,
>> > voltage-level and DBUF setup for mode X is already established in
>> > hardware. It isn't -- only firmware's own, entirely separate code
>> > path has ever touched those registers.
>> >
>> > The OS driver's first real modeset for an inherited pipe typically
>> > targets the same native panel mode, so the freshly computed
>> > crtc_state->min_cdclk/min_voltage_level trivially match this
>> > readout-seeded baseline. intel_cdclk_update_crtc_min_cdclk() and
>> > intel_cdclk_update_crtc_min_voltage_level() then conclude nothing
>> > changed and skip the recalculation, on the one commit where it
>> > actually matters: taking a pipe from firmware ownership to being
>> > correctly configured by this driver.
>> >
>> > Fix this in two parts:
>> >
>> >  - Invalidate the per-pipe min_cdclk[]/min_voltage_level[] tracking
>> >    right after boot-time readout, so the first real atomic commit is
>> >    guaranteed to see a difference.
>> >
>> >  - Stop early-returning in intel_cdclk_update_crtc_min_cdclk() and
>> >    intel_cdclk_update_crtc_min_voltage_level() based on the crtc_state
>> >    comparison alone. That comparison is unreliable for exactly the
>> >    same reason (it's derived from the same readout), and skips the
>> >    real, tracked-state check below it.
>> >
>> > Bisected on real hardware (Lenovo Yoga 9i 14IPH11, Panther Lake) down
>> > to a narrow window between v6.19.10 (clean on every cold boot) and
>> > v7.1.10 (broken on every cold boot); confirmed via live kernel
>> > tracing that the skip path fires unconditionally on this platform
>> > from the very first post-boot atomic commit onward. This fix
>> > eliminates the FIFO underrun across many consecutive cold boots on
>> > the same hardware, with no regression observed across suspend/resume.
>>
>> Do you have a regressing commit? Nearly 600 commits were merged to i915
>> display alone between 6.19 and 7.1 so it's not really a narrow window.
>>
>> Have you filed a bug as described at [1]? With debugs, logs, and
>> everything.
>>
>> Have you tried with 1786d2688781 ("drm/i915/cdclk: Avoid spurious cdclk
>> sanitization on PTL+")?
>>
>>
>> BR,
>> Jani.
>>
>>
>> [1]
>> https://drm.pages.freedesktop.org/intel-docs/how-to-file-i915-bugs.html
>>
>>
>> > The investigation and this fix were developed with the assistance of
>> > Claude (Anthropic), driven and verified end to end on the affected
>> > hardware by the Signed-off-by below.
>> >
>> > Signed-off-by: Eduardo Diaz <[email protected]>
>> > ---
>> >  drivers/gpu/drm/i915/display/intel_cdclk.c    | 45 ++++++++++++++-----
>> >  drivers/gpu/drm/i915/display/intel_cdclk.h    |  1 +
>> >  .../drm/i915/display/intel_modeset_setup.c    |  1 +
>> >  3 files changed, 36 insertions(+), 11 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c
>> b/drivers/gpu/drm/i915/display/intel_cdclk.c
>> > index a53d887271..209a2373d1 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
>> > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
>> > @@ -2981,11 +2981,14 @@ static int
>> intel_cdclk_update_crtc_min_cdclk(struct intel_atomic_state *state,
>> >       bool allow_cdclk_decrease = intel_any_crtc_needs_modeset(state);
>> >       int ret;
>> >
>> > -     if (new_min_cdclk == old_min_cdclk)
>> > -             return 0;
>> > -
>> > -     if (!allow_cdclk_decrease && new_min_cdclk < old_min_cdclk)
>> > -             return 0;
>> > +     /*
>> > +      * old_min_cdclk comes from the previous crtc_state, which after
>> > +      * boot-time readout reflects whatever firmware/GOP left running,
>> > +      * not what this driver has programmed. For an inherited pipe it
>> > +      * equals new_min_cdclk by construction (same mode, same formula),
>> > +      * so an early return here would skip the recalculation that
>> > +      * matters. Always continue on to the cdclk_state check below.
>> > +      */
>> >
>> >       cdclk_state = intel_atomic_get_cdclk_state(state);
>> >       if (IS_ERR(cdclk_state))
>> > @@ -3026,12 +3029,10 @@ static int
>> intel_cdclk_update_crtc_min_voltage_level(struct intel_atomic_state *
>> >       bool allow_voltage_level_decrease =
>> intel_any_crtc_needs_modeset(state);
>> >       int ret;
>> >
>> > -     if (new_min_voltage_level == old_min_voltage_level)
>> > -             return 0;
>> > -
>> > -     if (!allow_voltage_level_decrease &&
>> > -         new_min_voltage_level < old_min_voltage_level)
>> > -             return 0;
>> > +     /*
>> > +      * old_min_voltage_level is unreliable for the same reason; see
>> > +      * intel_cdclk_update_crtc_min_cdclk().
>> > +      */
>> >
>> >       cdclk_state = intel_atomic_get_cdclk_state(state);
>> >       if (IS_ERR(cdclk_state))
>> > @@ -3705,6 +3706,28 @@ void intel_cdclk_update_hw_state(struct
>> intel_display *display)
>> >       cdclk_state->dbuf_bw_min_cdclk = intel_dbuf_bw_min_cdclk(display,
>> dbuf_bw_state);
>> >  }
>> >
>> > +/*
>> > + * intel_cdclk_update_hw_state() seeds min_cdclk[]/min_voltage_level[]
>> > + * from readout's crtc_state, i.e. from whatever firmware/GOP left
>> > + * running, not from anything this driver has programmed. A pipe's
>> > + * first real modeset usually targets the same native mode, so the
>> > + * freshly computed value matches this seeded baseline and
>> > + * intel_cdclk_update_crtc_min_cdclk()/_min_voltage_level() conclude
>> > + * nothing changed, skipping the recalculation that matters. Call
>> > + * this after readout so the first real commit sees a difference.
>> > + */
>> > +void intel_cdclk_invalidate_min_tracking(struct intel_display *display)
>> > +{
>> > +     struct intel_cdclk_state *cdclk_state =
>> > +             to_intel_cdclk_state(display->cdclk.obj.state);
>> > +     enum pipe pipe;
>> > +
>> > +     for_each_pipe(display, pipe) {
>> > +             cdclk_state->min_cdclk[pipe] = 0;
>> > +             cdclk_state->min_voltage_level[pipe] = 0;
>> > +     }
>> > +}
>> > +
>> >  void intel_cdclk_crtc_disable_noatomic(struct intel_crtc *crtc)
>> >  {
>> >       struct intel_display *display = to_intel_display(crtc);
>> > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h
>> b/drivers/gpu/drm/i915/display/intel_cdclk.h
>> > index a60cbf745e..1517d3605a 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_cdclk.h
>> > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.h
>> > @@ -46,6 +46,7 @@ int intel_cdclk_state_set_joined_mbus(struct
>> intel_atomic_state *state, bool joi
>> >  struct intel_cdclk_state *
>> >  intel_atomic_get_cdclk_state(struct intel_atomic_state *state);
>> >  void intel_cdclk_update_hw_state(struct intel_display *display);
>> > +void intel_cdclk_invalidate_min_tracking(struct intel_display
>> *display);
>> >  void intel_cdclk_crtc_disable_noatomic(struct intel_crtc *crtc);
>> >  int intel_cdclk_update_dbuf_bw_min_cdclk(struct intel_atomic_state
>> *state,
>> >                                        int old_min_cdclk, int
>> new_min_cdclk,
>> > diff --git a/drivers/gpu/drm/i915/display/intel_modeset_setup.c
>> b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
>> > index 6aed881737..c7be6e63e7 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_modeset_setup.c
>> > +++ b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
>> > @@ -880,6 +880,7 @@ static void intel_modeset_readout_hw_state(struct
>> intel_display *display)
>> >       intel_bw_update_hw_state(display);
>> >       intel_dbuf_bw_update_hw_state(display);
>> >       intel_cdclk_update_hw_state(display);
>> > +     intel_cdclk_invalidate_min_tracking(display);
>> >
>> >       intel_pmdemand_init_pmdemand_params(display, pmdemand_state);
>> >  }
>>
>> --
>> Jani Nikula, Intel
>>
>

Reply via email to