[PATCH v4 0/2] Enable seamless boot (fastboot) for PTL

[PATCH v4 0/2] Enable seamless boot (fastboot) for PTL

[Juasheem Sultan]

This is the fourth version of a series of patches meant to add
support for seamless framebuffer handoff within the Xe driver. 
It was tested on Panther Lake platforms.

The goal of this series is to achieve a flicker-free transition from
the bootloader (BIOS/UEFI) to the kernel driver by strictly adhering
to the hardware state established by the firmware.

With this version, I've taken the feedback that the last version was
doing too broad of a copy of the hardware crtc state into the new 
atomic state. Rather than that, we are now instead sanitizing the 
clock values and pll state. 

The BIOS appears to set a slightly different clock than the ideal value
calculated by the driver. If these are within a small tolerance of each
other then we adopt the BIOS clock values. 

The pll state that the driver reads has bytes 4 to 8 programmed to 0
in non-ssc registers. The state read from the hardware doesn't have 
this, so we adopt the hardware state if they match without those bytes.

---
Changes since v1
- v2 Complete rewrite of the code
- v3 Resending due to failure of patches to send
- v4 Switched from complete state copy to clock sanitization

Juasheem Sultan (2):
  drm/xe/display: Fix reading the framebuffer from stolen memory
  drm/i915/display: Sync state to BIOS for seamless handoff

 drivers/gpu/drm/i915/display/intel_display.c  | 67 +++++++++++++++++++
 drivers/gpu/drm/xe/display/xe_initial_plane.c | 22 +++++-
 2 files changed, 88 insertions(+), 1 deletion(-)

-- 
2.53.0.851.ga537e3e6e9-goog

[PATCH v4 1/2] drm/xe/display: Fix reading the framebuffer from stolen memory

[Juasheem Sultan]

Currently, we attempt to pin stolen memory using the ggtt address. This
doesn't appear to actually read the framebuffer that was setup by the
bios. Instead, we have to use the underlying physical address offset
within stolen memory.

Signed-off-by: Juasheem Sultan <jdsultan@google.com>
---
 drivers/gpu/drm/xe/display/xe_initial_plane.c | 22 ++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/xe/display/xe_initial_plane.c b/drivers/gpu/drm/xe/display/xe_initial_plane.c
index 4cfeafcc158d..d818993d9b8a 100644
--- a/drivers/gpu/drm/xe/display/xe_initial_plane.c
+++ b/drivers/gpu/drm/xe/display/xe_initial_plane.c
@@ -19,6 +19,7 @@
 #include "intel_fb.h"
 #include "intel_fb_pin.h"
 #include "xe_bo.h"
+#include "xe_ttm_stolen_mgr.h"
 #include "xe_vram_types.h"
 #include "xe_wa.h"
 
@@ -87,7 +88,26 @@ initial_plane_bo(struct xe_device *xe,
 
 		if (!stolen)
 			return NULL;
-		phys_base = base;
+
+		/* Read PTE to find physical address backing the GGTT address */
+		u64 pte = xe_ggtt_read_pte(tile0->mem.ggtt, base);
+		u64 phys_addr = pte & ~(page_size - 1);
+
+		u64 stolen_base = xe_ttm_stolen_gpu_offset(xe);
+
+		drm_dbg_kms(&xe->drm,
+				"Stolen Framebuffer base=%x pte=%llx phys_addr=%llx stolen_base=%llx\n",
+				base, pte, phys_addr, stolen_base);
+
+		/* Make sure that the physical address is in the range of stolen memory */
+		if (phys_addr >= stolen_base) {
+			phys_base = phys_addr - stolen_base;
+		} else {
+			drm_err(&xe->drm, "Stolen memory outside of stolen range phys_base=%pa\n",
+					&phys_base);
+			return NULL;
+		}
+
 		flags |= XE_BO_FLAG_STOLEN;
 
 		if (XE_DEVICE_WA(xe, 22019338487_display))
-- 
2.53.0.851.ga537e3e6e9-goog

[PATCH v4 2/2] drm/i915/display: Sync state to BIOS for seamless handoff

[Juasheem Sultan]

Align DP timings and C10 PLL state with BIOS values if within a 0.5%
clock threshold. This prevents minor mismatches from triggering a full
modeset during the first atomic commit, ensuring a flicker-free handoff.

Signed-off-by: Juasheem Sultan <jdsultan@google.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 67 ++++++++++++++++++++
 1 file changed, 67 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index c4246481fc2f..22e5e931f134 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -6397,6 +6397,71 @@ static int intel_atomic_check_config_and_link(struct intel_atomic_state *state)
 
 	return ret;
 }
+
+// Helper function to sanitize pll state
+static void intel_sanitize_pll_state(struct intel_crtc_state *old_crtc_state,
+		struct intel_crtc_state *new_crtc_state)
+{
+	int j;
+
+	for (j = 4; j < 9; j++) {
+		if (new_crtc_state->dpll_hw_state.cx0pll.c10.pll[j] !=
+				old_crtc_state->dpll_hw_state.cx0pll.c10.pll[j]) {
+			new_crtc_state->dpll_hw_state.cx0pll.c10.pll[j] =
+				old_crtc_state->dpll_hw_state.cx0pll.c10.pll[j];
+		}
+	}
+}
+
+/*
+ * intel_dp_sanitize_seamless_boot - Snap driver state to BIOS state for seamless handoff.
+ * @state: the atomic state to sanitize
+ *
+ * This function compares the driver's calculated new_state with the inherited BIOS state
+ * (old_state). If they are within a small threshold (e.g., 0.5% for clock), it "snaps"
+ * the new_state to match the BIOS state exactly. This prevents minor state mismatches
+ * that would otherwise force a full modeset (and a screen flicker) during the initial
+ * kernel handoff.
+ */
+static void intel_dp_sanitize_seamless_boot(struct intel_atomic_state *state)
+{
+	struct intel_display *display = to_intel_display(state);
+	struct intel_crtc_state *new_crtc_state, *old_crtc_state;
+	struct intel_crtc *crtc;
+	struct intel_encoder *encoder;
+	int i;
+
+	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
+		/*
+		 * We must check old_crtc_state->inherited because new_crtc_state->inherited
+		 * is cleared at the start of intel_atomic_check for userspace commits.
+		 */
+		if (!old_crtc_state->inherited || !new_crtc_state->hw.active)
+			continue;
+
+		if (intel_crtc_has_dp_encoder(new_crtc_state)) {
+			int old_clock = old_crtc_state->hw.adjusted_mode.crtc_clock;
+			int new_clock = new_crtc_state->hw.adjusted_mode.crtc_clock;
+			int threshold = old_clock / 200; /* 0.5% */
+
+			if (abs(new_clock - old_clock) <= threshold) {
+				new_crtc_state->hw.pipe_mode.crtc_clock = old_clock;
+				new_crtc_state->hw.adjusted_mode.crtc_clock = old_clock;
+				new_crtc_state->pixel_rate = old_crtc_state->pixel_rate;
+				new_crtc_state->dp_m_n = old_crtc_state->dp_m_n;
+			}
+		}
+
+		for_each_intel_encoder_mask(display->drm, encoder,
+				new_crtc_state->uapi.encoder_mask) {
+			if (intel_encoder_is_c10phy(encoder)) {
+				if (!new_crtc_state->dpll_hw_state.cx0pll.ssc_enabled)
+					intel_sanitize_pll_state(old_crtc_state, new_crtc_state);
+			}
+		}
+	}
+}
+
 /**
  * intel_atomic_check - validate state object
  * @dev: drm device
@@ -6447,6 +6512,8 @@ int intel_atomic_check(struct drm_device *dev,
 	if (ret)
 		goto fail;
 
+	intel_dp_sanitize_seamless_boot(state);
+
 	for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
 		if (!intel_crtc_needs_modeset(new_crtc_state))
 			continue;
-- 
2.53.0.851.ga537e3e6e9-goog

Re: [PATCH v4 2/2] drm/i915/display: Sync state to BIOS for seamless handoff

[Maarten Lankhorst]

Hey,

Den 2026-03-17 kl. 23:09, skrev Juasheem Sultan:
> Align DP timings and C10 PLL state with BIOS values if within a 0.5%
> clock threshold. This prevents minor mismatches from triggering a full
> modeset during the first atomic commit, ensuring a flicker-free handoff.
> 
> Signed-off-by: Juasheem Sultan <jdsultan@google.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 67 ++++++++++++++++++++
>  1 file changed, 67 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index c4246481fc2f..22e5e931f134 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -6397,6 +6397,71 @@ static int intel_atomic_check_config_and_link(struct intel_atomic_state *state)
>  
>  	return ret;
>  }
> +
> +// Helper function to sanitize pll state
> +static void intel_sanitize_pll_state(struct intel_crtc_state *old_crtc_state,
> +		struct intel_crtc_state *new_crtc_state)
> +{
> +	int j;
> +
> +	for (j = 4; j < 9; j++) {
> +		if (new_crtc_state->dpll_hw_state.cx0pll.c10.pll[j] !=
> +				old_crtc_state->dpll_hw_state.cx0pll.c10.pll[j]) {
> +			new_crtc_state->dpll_hw_state.cx0pll.c10.pll[j] =
> +				old_crtc_state->dpll_hw_state.cx0pll.c10.pll[j];
> +		}
> +	}
> +}
> +
> +/*
> + * intel_dp_sanitize_seamless_boot - Snap driver state to BIOS state for seamless handoff.
> + * @state: the atomic state to sanitize
> + *
> + * This function compares the driver's calculated new_state with the inherited BIOS state
> + * (old_state). If they are within a small threshold (e.g., 0.5% for clock), it "snaps"
> + * the new_state to match the BIOS state exactly. This prevents minor state mismatches
> + * that would otherwise force a full modeset (and a screen flicker) during the initial
> + * kernel handoff.
> + */
> +static void intel_dp_sanitize_seamless_boot(struct intel_atomic_state *state)
> +{
> +	struct intel_display *display = to_intel_display(state);
> +	struct intel_crtc_state *new_crtc_state, *old_crtc_state;
> +	struct intel_crtc *crtc;
> +	struct intel_encoder *encoder;
> +	int i;
> +
> +	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
> +		/*
> +		 * We must check old_crtc_state->inherited because new_crtc_state->inherited
> +		 * is cleared at the start of intel_atomic_check for userspace commits.
> +		 */
> +		if (!old_crtc_state->inherited || !new_crtc_state->hw.active)
> +			continue;
> +
> +		if (intel_crtc_has_dp_encoder(new_crtc_state)) {
> +			int old_clock = old_crtc_state->hw.adjusted_mode.crtc_clock;
> +			int new_clock = new_crtc_state->hw.adjusted_mode.crtc_clock;
> +			int threshold = old_clock / 200; /* 0.5% */
> +
> +			if (abs(new_clock - old_clock) <= threshold) {
> +				new_crtc_state->hw.pipe_mode.crtc_clock = old_clock;
> +				new_crtc_state->hw.adjusted_mode.crtc_clock = old_clock;
> +				new_crtc_state->pixel_rate = old_crtc_state->pixel_rate;
> +				new_crtc_state->dp_m_n = old_crtc_state->dp_m_n;
> +			}
> +		}
> +
> +		for_each_intel_encoder_mask(display->drm, encoder,
> +				new_crtc_state->uapi.encoder_mask) {
> +			if (intel_encoder_is_c10phy(encoder)) {
> +				if (!new_crtc_state->dpll_hw_state.cx0pll.ssc_enabled)
> +					intel_sanitize_pll_state(old_crtc_state, new_crtc_state);
> +			}
> +		}
> +	}
> +}
> +
>  /**
>   * intel_atomic_check - validate state object
>   * @dev: drm device
> @@ -6447,6 +6512,8 @@ int intel_atomic_check(struct drm_device *dev,
>  	if (ret)
>  		goto fail;
>  
> +	intel_dp_sanitize_seamless_boot(state);
> +
>  	for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
>  		if (!intel_crtc_needs_modeset(new_crtc_state))
>  			continue;

This might fix boot state, but in a way that complicates the code considerably.

Have you considered updating intel_pipe_config_compare instead?

Kind regards,
~Maarten Lankhorst

Re: [PATCH v4 1/2] drm/xe/display: Fix reading the framebuffer from stolen memory

[Maarten Lankhorst]

Hey,

Den 2026-03-17 kl. 23:09, skrev Juasheem Sultan:
> Currently, we attempt to pin stolen memory using the ggtt address. This
> doesn't appear to actually read the framebuffer that was setup by the
> bios. Instead, we have to use the underlying physical address offset
> within stolen memory.
>
> Signed-off-by: Juasheem Sultan <jdsultan@google.com>
> ---
>  drivers/gpu/drm/xe/display/xe_initial_plane.c | 22 ++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/xe/display/xe_initial_plane.c b/drivers/gpu/drm/xe/display/xe_initial_plane.c
> index 4cfeafcc158d..d818993d9b8a 100644
> --- a/drivers/gpu/drm/xe/display/xe_initial_plane.c
> +++ b/drivers/gpu/drm/xe/display/xe_initial_plane.c
> @@ -19,6 +19,7 @@
>  #include "intel_fb.h"
>  #include "intel_fb_pin.h"
>  #include "xe_bo.h"
> +#include "xe_ttm_stolen_mgr.h"
>  #include "xe_vram_types.h"
>  #include "xe_wa.h"
>  
> @@ -87,7 +88,26 @@ initial_plane_bo(struct xe_device *xe,
>  
>  		if (!stolen)
>  			return NULL;
> -		phys_base = base;
> +
> +		/* Read PTE to find physical address backing the GGTT address */
> +		u64 pte = xe_ggtt_read_pte(tile0->mem.ggtt, base);
> +		u64 phys_addr = pte & ~(page_size - 1);
> +
> +		u64 stolen_base = xe_ttm_stolen_gpu_offset(xe);
> +
> +		drm_dbg_kms(&xe->drm,
> +				"Stolen Framebuffer base=%x pte=%llx phys_addr=%llx stolen_base=%llx\n",
> +				base, pte, phys_addr, stolen_base);
> +
> +		/* Make sure that the physical address is in the range of stolen memory */
> +		if (phys_addr >= stolen_base) {
> +			phys_base = phys_addr - stolen_base;
> +		} else {
> +			drm_err(&xe->drm, "Stolen memory outside of stolen range phys_base=%pa\n",
> +					&phys_base);
> +			return NULL;
> +		}
> +
>  		flags |= XE_BO_FLAG_STOLEN;
>  
>  		if (XE_DEVICE_WA(xe, 22019338487_display))
So far all platforms had an identity mapping of GGTT with stolen, is that different?

There should probably also be a mention that allocation will fail anyway if phys_addr + bo_size < xe_ttm_stolen_size()

Kind regards,
~Maarten Lankhorst

Re: [PATCH v4 2/2] drm/i915/display: Sync state to BIOS for seamless handoff

[Juasheem Sultan]

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

Hi,

Thanks for looking at this.

You're saying instead of manually adopting the state that I should focus on
modifying the comparisons that we do to determine if we can perform a
fastset?

-Juasheem

On Wed, Mar 18, 2026 at 5:00 AM Maarten Lankhorst <dev@lankhorst.se> wrote:

> Hey,
>
> Den 2026-03-17 kl. 23:09, skrev Juasheem Sultan:
> > Align DP timings and C10 PLL state with BIOS values if within a 0.5%
> > clock threshold. This prevents minor mismatches from triggering a full
> > modeset during the first atomic commit, ensuring a flicker-free handoff.
> >
> > Signed-off-by: Juasheem Sultan <jdsultan@google.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_display.c | 67 ++++++++++++++++++++
> >  1 file changed, 67 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> > index c4246481fc2f..22e5e931f134 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -6397,6 +6397,71 @@ static int
> intel_atomic_check_config_and_link(struct intel_atomic_state *state)
> >
> >       return ret;
> >  }
> > +
> > +// Helper function to sanitize pll state
> > +static void intel_sanitize_pll_state(struct intel_crtc_state
> *old_crtc_state,
> > +             struct intel_crtc_state *new_crtc_state)
> > +{
> > +     int j;
> > +
> > +     for (j = 4; j < 9; j++) {
> > +             if (new_crtc_state->dpll_hw_state.cx0pll.c10.pll[j] !=
> > +
>  old_crtc_state->dpll_hw_state.cx0pll.c10.pll[j]) {
> > +                     new_crtc_state->dpll_hw_state.cx0pll.c10.pll[j] =
> > +
>  old_crtc_state->dpll_hw_state.cx0pll.c10.pll[j];
> > +             }
> > +     }
> > +}
> > +
> > +/*
> > + * intel_dp_sanitize_seamless_boot - Snap driver state to BIOS state
> for seamless handoff.
> > + * @state: the atomic state to sanitize
> > + *
> > + * This function compares the driver's calculated new_state with the
> inherited BIOS state
> > + * (old_state). If they are within a small threshold (e.g., 0.5% for
> clock), it "snaps"
> > + * the new_state to match the BIOS state exactly. This prevents minor
> state mismatches
> > + * that would otherwise force a full modeset (and a screen flicker)
> during the initial
> > + * kernel handoff.
> > + */
> > +static void intel_dp_sanitize_seamless_boot(struct intel_atomic_state
> *state)
> > +{
> > +     struct intel_display *display = to_intel_display(state);
> > +     struct intel_crtc_state *new_crtc_state, *old_crtc_state;
> > +     struct intel_crtc *crtc;
> > +     struct intel_encoder *encoder;
> > +     int i;
> > +
> > +     for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
> new_crtc_state, i) {
> > +             /*
> > +              * We must check old_crtc_state->inherited because
> new_crtc_state->inherited
> > +              * is cleared at the start of intel_atomic_check for
> userspace commits.
> > +              */
> > +             if (!old_crtc_state->inherited ||
> !new_crtc_state->hw.active)
> > +                     continue;
> > +
> > +             if (intel_crtc_has_dp_encoder(new_crtc_state)) {
> > +                     int old_clock =
> old_crtc_state->hw.adjusted_mode.crtc_clock;
> > +                     int new_clock =
> new_crtc_state->hw.adjusted_mode.crtc_clock;
> > +                     int threshold = old_clock / 200; /* 0.5% */
> > +
> > +                     if (abs(new_clock - old_clock) <= threshold) {
> > +                             new_crtc_state->hw.pipe_mode.crtc_clock =
> old_clock;
> > +
>  new_crtc_state->hw.adjusted_mode.crtc_clock = old_clock;
> > +                             new_crtc_state->pixel_rate =
> old_crtc_state->pixel_rate;
> > +                             new_crtc_state->dp_m_n =
> old_crtc_state->dp_m_n;
> > +                     }
> > +             }
> > +
> > +             for_each_intel_encoder_mask(display->drm, encoder,
> > +                             new_crtc_state->uapi.encoder_mask) {
> > +                     if (intel_encoder_is_c10phy(encoder)) {
> > +                             if
> (!new_crtc_state->dpll_hw_state.cx0pll.ssc_enabled)
> > +
>  intel_sanitize_pll_state(old_crtc_state, new_crtc_state);
> > +                     }
> > +             }
> > +     }
> > +}
> > +
> >  /**
> >   * intel_atomic_check - validate state object
> >   * @dev: drm device
> > @@ -6447,6 +6512,8 @@ int intel_atomic_check(struct drm_device *dev,
> >       if (ret)
> >               goto fail;
> >
> > +     intel_dp_sanitize_seamless_boot(state);
> > +
> >       for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
> >               if (!intel_crtc_needs_modeset(new_crtc_state))
> >                       continue;
>
> This might fix boot state, but in a way that complicates the code
> considerably.
>
> Have you considered updating intel_pipe_config_compare instead?
>
> Kind regards,
> ~Maarten Lankhorst
>

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

Re: [PATCH v4 2/2] drm/i915/display: Sync state to BIOS for seamless handoff

[Maarten Lankhorst]

Hey,

Den 2026-03-27 kl. 00:10, skrev Juasheem Sultan:
> Hi, 
> 
> Thanks for looking at this. 
> 
> You're saying instead of manually adopting the state that I should focus on modifying the comparisons that we do to determine if we can perform a fastset? 
That's it exactly, that's the place where those comparisons are relaxed when we inherit the boot state.

Kind regards,
~Maarten Lankhorst

> 
> -Juasheem
> 
> On Wed, Mar 18, 2026 at 5:00 AM Maarten Lankhorst <dev@lankhorst.se <mailto:dev@lankhorst.se>> wrote:
> 
>     Hey,
> 
>     Den 2026-03-17 kl. 23:09, skrev Juasheem Sultan:
>     > Align DP timings and C10 PLL state with BIOS values if within a 0.5%
>     > clock threshold. This prevents minor mismatches from triggering a full
>     > modeset during the first atomic commit, ensuring a flicker-free handoff.
>     >
>     > Signed-off-by: Juasheem Sultan <jdsultan@google.com <mailto:jdsultan@google.com>>
>     > ---
>     >  drivers/gpu/drm/i915/display/intel_display.c | 67 ++++++++++++++++++++
>     >  1 file changed, 67 insertions(+)
>     >
>     > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
>     > index c4246481fc2f..22e5e931f134 100644
>     > --- a/drivers/gpu/drm/i915/display/intel_display.c
>     > +++ b/drivers/gpu/drm/i915/display/intel_display.c
>     > @@ -6397,6 +6397,71 @@ static int intel_atomic_check_config_and_link(struct intel_atomic_state *state)
>     > 
>     >       return ret;
>     >  }
>     > +
>     > +// Helper function to sanitize pll state
>     > +static void intel_sanitize_pll_state(struct intel_crtc_state *old_crtc_state,
>     > +             struct intel_crtc_state *new_crtc_state)
>     > +{
>     > +     int j;
>     > +
>     > +     for (j = 4; j < 9; j++) {
>     > +             if (new_crtc_state->dpll_hw_state.cx0pll.c10.pll[j] !=
>     > +                             old_crtc_state->dpll_hw_state.cx0pll.c10.pll[j]) {
>     > +                     new_crtc_state->dpll_hw_state.cx0pll.c10.pll[j] =
>     > +                             old_crtc_state->dpll_hw_state.cx0pll.c10.pll[j];
>     > +             }
>     > +     }
>     > +}
>     > +
>     > +/*
>     > + * intel_dp_sanitize_seamless_boot - Snap driver state to BIOS state for seamless handoff.
>     > + * @state: the atomic state to sanitize
>     > + *
>     > + * This function compares the driver's calculated new_state with the inherited BIOS state
>     > + * (old_state). If they are within a small threshold (e.g., 0.5% for clock), it "snaps"
>     > + * the new_state to match the BIOS state exactly. This prevents minor state mismatches
>     > + * that would otherwise force a full modeset (and a screen flicker) during the initial
>     > + * kernel handoff.
>     > + */
>     > +static void intel_dp_sanitize_seamless_boot(struct intel_atomic_state *state)
>     > +{
>     > +     struct intel_display *display = to_intel_display(state);
>     > +     struct intel_crtc_state *new_crtc_state, *old_crtc_state;
>     > +     struct intel_crtc *crtc;
>     > +     struct intel_encoder *encoder;
>     > +     int i;
>     > +
>     > +     for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
>     > +             /*
>     > +              * We must check old_crtc_state->inherited because new_crtc_state->inherited
>     > +              * is cleared at the start of intel_atomic_check for userspace commits.
>     > +              */
>     > +             if (!old_crtc_state->inherited || !new_crtc_state->hw.active)
>     > +                     continue;
>     > +
>     > +             if (intel_crtc_has_dp_encoder(new_crtc_state)) {
>     > +                     int old_clock = old_crtc_state->hw.adjusted_mode.crtc_clock;
>     > +                     int new_clock = new_crtc_state->hw.adjusted_mode.crtc_clock;
>     > +                     int threshold = old_clock / 200; /* 0.5% */
>     > +
>     > +                     if (abs(new_clock - old_clock) <= threshold) {
>     > +                             new_crtc_state->hw.pipe_mode.crtc_clock = old_clock;
>     > +                             new_crtc_state->hw.adjusted_mode.crtc_clock = old_clock;
>     > +                             new_crtc_state->pixel_rate = old_crtc_state->pixel_rate;
>     > +                             new_crtc_state->dp_m_n = old_crtc_state->dp_m_n;
>     > +                     }
>     > +             }
>     > +
>     > +             for_each_intel_encoder_mask(display->drm, encoder,
>     > +                             new_crtc_state->uapi.encoder_mask) {
>     > +                     if (intel_encoder_is_c10phy(encoder)) {
>     > +                             if (!new_crtc_state->dpll_hw_state.cx0pll.ssc_enabled)
>     > +                                     intel_sanitize_pll_state(old_crtc_state, new_crtc_state);
>     > +                     }
>     > +             }
>     > +     }
>     > +}
>     > +
>     >  /**
>     >   * intel_atomic_check - validate state object
>     >   * @dev: drm device
>     > @@ -6447,6 +6512,8 @@ int intel_atomic_check(struct drm_device *dev,
>     >       if (ret)
>     >               goto fail;
>     > 
>     > +     intel_dp_sanitize_seamless_boot(state);
>     > +
>     >       for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
>     >               if (!intel_crtc_needs_modeset(new_crtc_state))
>     >                       continue;
> 
>     This might fix boot state, but in a way that complicates the code considerably.
> 
>     Have you considered updating intel_pipe_config_compare instead?
> 
>     Kind regards,
>     ~Maarten Lankhorst
> 

Re: [PATCH v4 2/2] drm/i915/display: Sync state to BIOS for seamless handoff

[Ville Syrjälä]

On Thu, Mar 26, 2026 at 04:10:13PM -0700, Juasheem Sultan wrote:
> Hi,
> 
> Thanks for looking at this.
> 
> You're saying instead of manually adopting the state that I should focus on
> modifying the comparisons that we do to determine if we can perform a
> fastset?

We don't want any fuzzy fastset hacks anywhere. I intentionally killed
all that stuff because it was making it impossible to trust that the
software state actually represents what the hardware is doing.

Someone needs to figure out what exactly is the difference between
the states between the GOP and the driver, and then figure out where
that difference is coming from.

> 
> -Juasheem
> 
> On Wed, Mar 18, 2026 at 5:00 AM Maarten Lankhorst <dev@lankhorst.se> wrote:
> 
> > Hey,
> >
> > Den 2026-03-17 kl. 23:09, skrev Juasheem Sultan:
> > > Align DP timings and C10 PLL state with BIOS values if within a 0.5%
> > > clock threshold. This prevents minor mismatches from triggering a full
> > > modeset during the first atomic commit, ensuring a flicker-free handoff.
> > >
> > > Signed-off-by: Juasheem Sultan <jdsultan@google.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_display.c | 67 ++++++++++++++++++++
> > >  1 file changed, 67 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > b/drivers/gpu/drm/i915/display/intel_display.c
> > > index c4246481fc2f..22e5e931f134 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > @@ -6397,6 +6397,71 @@ static int
> > intel_atomic_check_config_and_link(struct intel_atomic_state *state)
> > >
> > >       return ret;
> > >  }
> > > +
> > > +// Helper function to sanitize pll state
> > > +static void intel_sanitize_pll_state(struct intel_crtc_state
> > *old_crtc_state,
> > > +             struct intel_crtc_state *new_crtc_state)
> > > +{
> > > +     int j;
> > > +
> > > +     for (j = 4; j < 9; j++) {
> > > +             if (new_crtc_state->dpll_hw_state.cx0pll.c10.pll[j] !=
> > > +
> >  old_crtc_state->dpll_hw_state.cx0pll.c10.pll[j]) {
> > > +                     new_crtc_state->dpll_hw_state.cx0pll.c10.pll[j] =
> > > +
> >  old_crtc_state->dpll_hw_state.cx0pll.c10.pll[j];
> > > +             }
> > > +     }
> > > +}
> > > +
> > > +/*
> > > + * intel_dp_sanitize_seamless_boot - Snap driver state to BIOS state
> > for seamless handoff.
> > > + * @state: the atomic state to sanitize
> > > + *
> > > + * This function compares the driver's calculated new_state with the
> > inherited BIOS state
> > > + * (old_state). If they are within a small threshold (e.g., 0.5% for
> > clock), it "snaps"
> > > + * the new_state to match the BIOS state exactly. This prevents minor
> > state mismatches
> > > + * that would otherwise force a full modeset (and a screen flicker)
> > during the initial
> > > + * kernel handoff.
> > > + */
> > > +static void intel_dp_sanitize_seamless_boot(struct intel_atomic_state
> > *state)
> > > +{
> > > +     struct intel_display *display = to_intel_display(state);
> > > +     struct intel_crtc_state *new_crtc_state, *old_crtc_state;
> > > +     struct intel_crtc *crtc;
> > > +     struct intel_encoder *encoder;
> > > +     int i;
> > > +
> > > +     for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
> > new_crtc_state, i) {
> > > +             /*
> > > +              * We must check old_crtc_state->inherited because
> > new_crtc_state->inherited
> > > +              * is cleared at the start of intel_atomic_check for
> > userspace commits.
> > > +              */
> > > +             if (!old_crtc_state->inherited ||
> > !new_crtc_state->hw.active)
> > > +                     continue;
> > > +
> > > +             if (intel_crtc_has_dp_encoder(new_crtc_state)) {
> > > +                     int old_clock =
> > old_crtc_state->hw.adjusted_mode.crtc_clock;
> > > +                     int new_clock =
> > new_crtc_state->hw.adjusted_mode.crtc_clock;
> > > +                     int threshold = old_clock / 200; /* 0.5% */
> > > +
> > > +                     if (abs(new_clock - old_clock) <= threshold) {
> > > +                             new_crtc_state->hw.pipe_mode.crtc_clock =
> > old_clock;
> > > +
> >  new_crtc_state->hw.adjusted_mode.crtc_clock = old_clock;
> > > +                             new_crtc_state->pixel_rate =
> > old_crtc_state->pixel_rate;
> > > +                             new_crtc_state->dp_m_n =
> > old_crtc_state->dp_m_n;
> > > +                     }
> > > +             }
> > > +
> > > +             for_each_intel_encoder_mask(display->drm, encoder,
> > > +                             new_crtc_state->uapi.encoder_mask) {
> > > +                     if (intel_encoder_is_c10phy(encoder)) {
> > > +                             if
> > (!new_crtc_state->dpll_hw_state.cx0pll.ssc_enabled)
> > > +
> >  intel_sanitize_pll_state(old_crtc_state, new_crtc_state);
> > > +                     }
> > > +             }
> > > +     }
> > > +}
> > > +
> > >  /**
> > >   * intel_atomic_check - validate state object
> > >   * @dev: drm device
> > > @@ -6447,6 +6512,8 @@ int intel_atomic_check(struct drm_device *dev,
> > >       if (ret)
> > >               goto fail;
> > >
> > > +     intel_dp_sanitize_seamless_boot(state);
> > > +
> > >       for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
> > >               if (!intel_crtc_needs_modeset(new_crtc_state))
> > >                       continue;
> >
> > This might fix boot state, but in a way that complicates the code
> > considerably.
> >
> > Have you considered updating intel_pipe_config_compare instead?
> >
> > Kind regards,
> > ~Maarten Lankhorst
> >

-- 
Ville Syrjälä
Intel

Re: [PATCH v4 2/2] drm/i915/display: Sync state to BIOS for seamless handoff

[Maarten Lankhorst]

Hey,

Den 2026-03-27 kl. 11:11, skrev Ville Syrjälä:
> On Thu, Mar 26, 2026 at 04:10:13PM -0700, Juasheem Sultan wrote:
>> Hi,
>>
>> Thanks for looking at this.
>>
>> You're saying instead of manually adopting the state that I should focus on
>> modifying the comparisons that we do to determine if we can perform a
>> fastset?
> 
> We don't want any fuzzy fastset hacks anywhere. I intentionally killed
> all that stuff because it was making it impossible to trust that the
> software state actually represents what the hardware is doing.
> 
> Someone needs to figure out what exactly is the difference between
> the states between the GOP and the driver, and then figure out where
> that difference is coming from.

In some cases the firmware may have different considerations for setting a mode,
not necesssarily worse than what the display driver is using, just different.

In this case userspace requests a mode for example 1920x1080, but it doesn't
particularly care what pll's are used or anything nor has it a way to specify
it at all.

Until there is a major change like disabling the screen, what are the downsides
taking over the inherited mode from the firmware, and postponing changing the
display mode?

Having a flicker-free boot is a desired feature to have, and something intel
handles a lot better than the other drivers.

Kind regards,
~Maarten Lankhorst

>>
>> -Juasheem
>>
>> On Wed, Mar 18, 2026 at 5:00 AM Maarten Lankhorst <dev@lankhorst.se> wrote:
>>
>>> Hey,
>>>
>>> Den 2026-03-17 kl. 23:09, skrev Juasheem Sultan:
>>>> Align DP timings and C10 PLL state with BIOS values if within a 0.5%
>>>> clock threshold. This prevents minor mismatches from triggering a full
>>>> modeset during the first atomic commit, ensuring a flicker-free handoff.
>>>>
>>>> Signed-off-by: Juasheem Sultan <jdsultan@google.com>
>>>> ---
>>>>  drivers/gpu/drm/i915/display/intel_display.c | 67 ++++++++++++++++++++
>>>>  1 file changed, 67 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
>>> b/drivers/gpu/drm/i915/display/intel_display.c
>>>> index c4246481fc2f..22e5e931f134 100644
>>>> --- a/drivers/gpu/drm/i915/display/intel_display.c
>>>> +++ b/drivers/gpu/drm/i915/display/intel_display.c
>>>> @@ -6397,6 +6397,71 @@ static int
>>> intel_atomic_check_config_and_link(struct intel_atomic_state *state)
>>>>
>>>>       return ret;
>>>>  }
>>>> +
>>>> +// Helper function to sanitize pll state
>>>> +static void intel_sanitize_pll_state(struct intel_crtc_state
>>> *old_crtc_state,
>>>> +             struct intel_crtc_state *new_crtc_state)
>>>> +{
>>>> +     int j;
>>>> +
>>>> +     for (j = 4; j < 9; j++) {
>>>> +             if (new_crtc_state->dpll_hw_state.cx0pll.c10.pll[j] !=
>>>> +
>>>  old_crtc_state->dpll_hw_state.cx0pll.c10.pll[j]) {
>>>> +                     new_crtc_state->dpll_hw_state.cx0pll.c10.pll[j] =
>>>> +
>>>  old_crtc_state->dpll_hw_state.cx0pll.c10.pll[j];
>>>> +             }
>>>> +     }
>>>> +}
>>>> +
>>>> +/*
>>>> + * intel_dp_sanitize_seamless_boot - Snap driver state to BIOS state
>>> for seamless handoff.
>>>> + * @state: the atomic state to sanitize
>>>> + *
>>>> + * This function compares the driver's calculated new_state with the
>>> inherited BIOS state
>>>> + * (old_state). If they are within a small threshold (e.g., 0.5% for
>>> clock), it "snaps"
>>>> + * the new_state to match the BIOS state exactly. This prevents minor
>>> state mismatches
>>>> + * that would otherwise force a full modeset (and a screen flicker)
>>> during the initial
>>>> + * kernel handoff.
>>>> + */
>>>> +static void intel_dp_sanitize_seamless_boot(struct intel_atomic_state
>>> *state)
>>>> +{
>>>> +     struct intel_display *display = to_intel_display(state);
>>>> +     struct intel_crtc_state *new_crtc_state, *old_crtc_state;
>>>> +     struct intel_crtc *crtc;
>>>> +     struct intel_encoder *encoder;
>>>> +     int i;
>>>> +
>>>> +     for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
>>> new_crtc_state, i) {
>>>> +             /*
>>>> +              * We must check old_crtc_state->inherited because
>>> new_crtc_state->inherited
>>>> +              * is cleared at the start of intel_atomic_check for
>>> userspace commits.
>>>> +              */
>>>> +             if (!old_crtc_state->inherited ||
>>> !new_crtc_state->hw.active)
>>>> +                     continue;
>>>> +
>>>> +             if (intel_crtc_has_dp_encoder(new_crtc_state)) {
>>>> +                     int old_clock =
>>> old_crtc_state->hw.adjusted_mode.crtc_clock;
>>>> +                     int new_clock =
>>> new_crtc_state->hw.adjusted_mode.crtc_clock;
>>>> +                     int threshold = old_clock / 200; /* 0.5% */
>>>> +
>>>> +                     if (abs(new_clock - old_clock) <= threshold) {
>>>> +                             new_crtc_state->hw.pipe_mode.crtc_clock =
>>> old_clock;
>>>> +
>>>  new_crtc_state->hw.adjusted_mode.crtc_clock = old_clock;
>>>> +                             new_crtc_state->pixel_rate =
>>> old_crtc_state->pixel_rate;
>>>> +                             new_crtc_state->dp_m_n =
>>> old_crtc_state->dp_m_n;
>>>> +                     }
>>>> +             }
>>>> +
>>>> +             for_each_intel_encoder_mask(display->drm, encoder,
>>>> +                             new_crtc_state->uapi.encoder_mask) {
>>>> +                     if (intel_encoder_is_c10phy(encoder)) {
>>>> +                             if
>>> (!new_crtc_state->dpll_hw_state.cx0pll.ssc_enabled)
>>>> +
>>>  intel_sanitize_pll_state(old_crtc_state, new_crtc_state);
>>>> +                     }
>>>> +             }
>>>> +     }
>>>> +}
>>>> +
>>>>  /**
>>>>   * intel_atomic_check - validate state object
>>>>   * @dev: drm device
>>>> @@ -6447,6 +6512,8 @@ int intel_atomic_check(struct drm_device *dev,
>>>>       if (ret)
>>>>               goto fail;
>>>>
>>>> +     intel_dp_sanitize_seamless_boot(state);
>>>> +
>>>>       for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
>>>>               if (!intel_crtc_needs_modeset(new_crtc_state))
>>>>                       continue;
>>>
>>> This might fix boot state, but in a way that complicates the code
>>> considerably.
>>>
>>> Have you considered updating intel_pipe_config_compare instead?
>>>
>>> Kind regards,
>>> ~Maarten Lankhorst
>>>
> 

Re: [PATCH v4 2/2] drm/i915/display: Sync state to BIOS for seamless handoff

[Ville Syrjälä]

On Wed, Apr 01, 2026 at 06:19:00PM +0200, Maarten Lankhorst wrote:
> Hey,
> 
> Den 2026-03-27 kl. 11:11, skrev Ville Syrjälä:
> > On Thu, Mar 26, 2026 at 04:10:13PM -0700, Juasheem Sultan wrote:
> >> Hi,
> >>
> >> Thanks for looking at this.
> >>
> >> You're saying instead of manually adopting the state that I should focus on
> >> modifying the comparisons that we do to determine if we can perform a
> >> fastset?
> > 
> > We don't want any fuzzy fastset hacks anywhere. I intentionally killed
> > all that stuff because it was making it impossible to trust that the
> > software state actually represents what the hardware is doing.
> > 
> > Someone needs to figure out what exactly is the difference between
> > the states between the GOP and the driver, and then figure out where
> > that difference is coming from.
> 
> In some cases the firmware may have different considerations for setting a mode,
> not necesssarily worse than what the display driver is using, just different.
> 
> In this case userspace requests a mode for example 1920x1080, but it doesn't
> particularly care what pll's are used or anything nor has it a way to specify
> it at all.
> 
> Until there is a major change like disabling the screen, what are the downsides
> taking over the inherited mode from the firmware, and postponing changing the
> display mode?

Because all the hacks we had for this of the form:
1. fully compute a new state
2. randomly overwrite bits and pieces of that with the 
   old state
3. somehow hope that whatever got overwritten isn't incompatible
   with all the stuff that wasn't overwritten

It was a completely unmaintainable mess that could result in all
kinds of weird bugs.

And to be clear, we *do* take the state the firmware was using, and
ideally we don't recompute it at all. It's only because we lack certain
readout support/etc that we sometimes punt to the full computation and
therefore we may end up with a fastset mismatch (see the
.initial_fastset_check() stuff). The correct way to solve that is to
add the missing readout support, not add more hacks.

> 
> Having a flicker-free boot is a desired feature to have, and something intel
> handles a lot better than the other drivers.
> 
> Kind regards,
> ~Maarten Lankhorst
> 
> >>
> >> -Juasheem
> >>
> >> On Wed, Mar 18, 2026 at 5:00 AM Maarten Lankhorst <dev@lankhorst.se> wrote:
> >>
> >>> Hey,
> >>>
> >>> Den 2026-03-17 kl. 23:09, skrev Juasheem Sultan:
> >>>> Align DP timings and C10 PLL state with BIOS values if within a 0.5%
> >>>> clock threshold. This prevents minor mismatches from triggering a full
> >>>> modeset during the first atomic commit, ensuring a flicker-free handoff.
> >>>>
> >>>> Signed-off-by: Juasheem Sultan <jdsultan@google.com>
> >>>> ---
> >>>>  drivers/gpu/drm/i915/display/intel_display.c | 67 ++++++++++++++++++++
> >>>>  1 file changed, 67 insertions(+)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> >>> b/drivers/gpu/drm/i915/display/intel_display.c
> >>>> index c4246481fc2f..22e5e931f134 100644
> >>>> --- a/drivers/gpu/drm/i915/display/intel_display.c
> >>>> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> >>>> @@ -6397,6 +6397,71 @@ static int
> >>> intel_atomic_check_config_and_link(struct intel_atomic_state *state)
> >>>>
> >>>>       return ret;
> >>>>  }
> >>>> +
> >>>> +// Helper function to sanitize pll state
> >>>> +static void intel_sanitize_pll_state(struct intel_crtc_state
> >>> *old_crtc_state,
> >>>> +             struct intel_crtc_state *new_crtc_state)
> >>>> +{
> >>>> +     int j;
> >>>> +
> >>>> +     for (j = 4; j < 9; j++) {
> >>>> +             if (new_crtc_state->dpll_hw_state.cx0pll.c10.pll[j] !=
> >>>> +
> >>>  old_crtc_state->dpll_hw_state.cx0pll.c10.pll[j]) {
> >>>> +                     new_crtc_state->dpll_hw_state.cx0pll.c10.pll[j] =
> >>>> +
> >>>  old_crtc_state->dpll_hw_state.cx0pll.c10.pll[j];
> >>>> +             }
> >>>> +     }
> >>>> +}
> >>>> +
> >>>> +/*
> >>>> + * intel_dp_sanitize_seamless_boot - Snap driver state to BIOS state
> >>> for seamless handoff.
> >>>> + * @state: the atomic state to sanitize
> >>>> + *
> >>>> + * This function compares the driver's calculated new_state with the
> >>> inherited BIOS state
> >>>> + * (old_state). If they are within a small threshold (e.g., 0.5% for
> >>> clock), it "snaps"
> >>>> + * the new_state to match the BIOS state exactly. This prevents minor
> >>> state mismatches
> >>>> + * that would otherwise force a full modeset (and a screen flicker)
> >>> during the initial
> >>>> + * kernel handoff.
> >>>> + */
> >>>> +static void intel_dp_sanitize_seamless_boot(struct intel_atomic_state
> >>> *state)
> >>>> +{
> >>>> +     struct intel_display *display = to_intel_display(state);
> >>>> +     struct intel_crtc_state *new_crtc_state, *old_crtc_state;
> >>>> +     struct intel_crtc *crtc;
> >>>> +     struct intel_encoder *encoder;
> >>>> +     int i;
> >>>> +
> >>>> +     for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
> >>> new_crtc_state, i) {
> >>>> +             /*
> >>>> +              * We must check old_crtc_state->inherited because
> >>> new_crtc_state->inherited
> >>>> +              * is cleared at the start of intel_atomic_check for
> >>> userspace commits.
> >>>> +              */
> >>>> +             if (!old_crtc_state->inherited ||
> >>> !new_crtc_state->hw.active)
> >>>> +                     continue;
> >>>> +
> >>>> +             if (intel_crtc_has_dp_encoder(new_crtc_state)) {
> >>>> +                     int old_clock =
> >>> old_crtc_state->hw.adjusted_mode.crtc_clock;
> >>>> +                     int new_clock =
> >>> new_crtc_state->hw.adjusted_mode.crtc_clock;
> >>>> +                     int threshold = old_clock / 200; /* 0.5% */
> >>>> +
> >>>> +                     if (abs(new_clock - old_clock) <= threshold) {
> >>>> +                             new_crtc_state->hw.pipe_mode.crtc_clock =
> >>> old_clock;
> >>>> +
> >>>  new_crtc_state->hw.adjusted_mode.crtc_clock = old_clock;
> >>>> +                             new_crtc_state->pixel_rate =
> >>> old_crtc_state->pixel_rate;
> >>>> +                             new_crtc_state->dp_m_n =
> >>> old_crtc_state->dp_m_n;
> >>>> +                     }
> >>>> +             }
> >>>> +
> >>>> +             for_each_intel_encoder_mask(display->drm, encoder,
> >>>> +                             new_crtc_state->uapi.encoder_mask) {
> >>>> +                     if (intel_encoder_is_c10phy(encoder)) {
> >>>> +                             if
> >>> (!new_crtc_state->dpll_hw_state.cx0pll.ssc_enabled)
> >>>> +
> >>>  intel_sanitize_pll_state(old_crtc_state, new_crtc_state);
> >>>> +                     }
> >>>> +             }
> >>>> +     }
> >>>> +}
> >>>> +
> >>>>  /**
> >>>>   * intel_atomic_check - validate state object
> >>>>   * @dev: drm device
> >>>> @@ -6447,6 +6512,8 @@ int intel_atomic_check(struct drm_device *dev,
> >>>>       if (ret)
> >>>>               goto fail;
> >>>>
> >>>> +     intel_dp_sanitize_seamless_boot(state);
> >>>> +
> >>>>       for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
> >>>>               if (!intel_crtc_needs_modeset(new_crtc_state))
> >>>>                       continue;
> >>>
> >>> This might fix boot state, but in a way that complicates the code
> >>> considerably.
> >>>
> >>> Have you considered updating intel_pipe_config_compare instead?
> >>>
> >>> Kind regards,
> >>> ~Maarten Lankhorst
> >>>
> > 

-- 
Ville Syrjälä
Intel

Re: [PATCH v4 2/2] drm/i915/display: Sync state to BIOS for seamless handoff

[Ville Syrjälä]

On Wed, Apr 01, 2026 at 08:44:32PM +0300, Ville Syrjälä wrote:
> On Wed, Apr 01, 2026 at 06:19:00PM +0200, Maarten Lankhorst wrote:
> > Hey,
> > 
> > Den 2026-03-27 kl. 11:11, skrev Ville Syrjälä:
> > > On Thu, Mar 26, 2026 at 04:10:13PM -0700, Juasheem Sultan wrote:
> > >> Hi,
> > >>
> > >> Thanks for looking at this.
> > >>
> > >> You're saying instead of manually adopting the state that I should focus on
> > >> modifying the comparisons that we do to determine if we can perform a
> > >> fastset?
> > > 
> > > We don't want any fuzzy fastset hacks anywhere. I intentionally killed
> > > all that stuff because it was making it impossible to trust that the
> > > software state actually represents what the hardware is doing.
> > > 
> > > Someone needs to figure out what exactly is the difference between
> > > the states between the GOP and the driver, and then figure out where
> > > that difference is coming from.
> > 
> > In some cases the firmware may have different considerations for setting a mode,
> > not necesssarily worse than what the display driver is using, just different.
> > 
> > In this case userspace requests a mode for example 1920x1080, but it doesn't
> > particularly care what pll's are used or anything nor has it a way to specify
> > it at all.
> > 
> > Until there is a major change like disabling the screen, what are the downsides
> > taking over the inherited mode from the firmware, and postponing changing the
> > display mode?
> 
> Because all the hacks we had for this of the form:
> 1. fully compute a new state
> 2. randomly overwrite bits and pieces of that with the 
>    old state
> 3. somehow hope that whatever got overwritten isn't incompatible
>    with all the stuff that wasn't overwritten
> 
> It was a completely unmaintainable mess that could result in all
> kinds of weird bugs.
> 
> And to be clear, we *do* take the state the firmware was using, and
> ideally we don't recompute it at all. It's only because we lack certain
> readout support/etc that we sometimes punt to the full computation and
> therefore we may end up with a fastset mismatch (see the
> .initial_fastset_check() stuff). The correct way to solve that is to
> add the missing readout support, not add more hacks.

I suppose I should say that only really covers in the initial_commit().

I do admit that we may have a bit of a problem with the initial
userspace commit (when crtc_state->inherited is cleared). In the past
that was required at least to fix audio, but audio shouldn't really need
that anymore since I added fastset support for it.

What could be attempted is to move the audio state computation into
.compute_config_late(), and if userspace didn't flag a modeset on the
first commit, then we'd skip the full .compute_config() and just do
the _late() part. Afterwards the normal audio fastset should kick in.
That's assuming audio isn't a dependency of some other stuff in
.compute_config().

And of course we'd have to think whether we have some other features
besides audio that users actually want to work, but we only
enable/disable them in response to the full .compute_config()...

> 
> > 
> > Having a flicker-free boot is a desired feature to have, and something intel
> > handles a lot better than the other drivers.
> > 
> > Kind regards,
> > ~Maarten Lankhorst
> > 
> > >>
> > >> -Juasheem
> > >>
> > >> On Wed, Mar 18, 2026 at 5:00 AM Maarten Lankhorst <dev@lankhorst.se> wrote:
> > >>
> > >>> Hey,
> > >>>
> > >>> Den 2026-03-17 kl. 23:09, skrev Juasheem Sultan:
> > >>>> Align DP timings and C10 PLL state with BIOS values if within a 0.5%
> > >>>> clock threshold. This prevents minor mismatches from triggering a full
> > >>>> modeset during the first atomic commit, ensuring a flicker-free handoff.
> > >>>>
> > >>>> Signed-off-by: Juasheem Sultan <jdsultan@google.com>
> > >>>> ---
> > >>>>  drivers/gpu/drm/i915/display/intel_display.c | 67 ++++++++++++++++++++
> > >>>>  1 file changed, 67 insertions(+)
> > >>>>
> > >>>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > >>> b/drivers/gpu/drm/i915/display/intel_display.c
> > >>>> index c4246481fc2f..22e5e931f134 100644
> > >>>> --- a/drivers/gpu/drm/i915/display/intel_display.c
> > >>>> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > >>>> @@ -6397,6 +6397,71 @@ static int
> > >>> intel_atomic_check_config_and_link(struct intel_atomic_state *state)
> > >>>>
> > >>>>       return ret;
> > >>>>  }
> > >>>> +
> > >>>> +// Helper function to sanitize pll state
> > >>>> +static void intel_sanitize_pll_state(struct intel_crtc_state
> > >>> *old_crtc_state,
> > >>>> +             struct intel_crtc_state *new_crtc_state)
> > >>>> +{
> > >>>> +     int j;
> > >>>> +
> > >>>> +     for (j = 4; j < 9; j++) {
> > >>>> +             if (new_crtc_state->dpll_hw_state.cx0pll.c10.pll[j] !=
> > >>>> +
> > >>>  old_crtc_state->dpll_hw_state.cx0pll.c10.pll[j]) {
> > >>>> +                     new_crtc_state->dpll_hw_state.cx0pll.c10.pll[j] =
> > >>>> +
> > >>>  old_crtc_state->dpll_hw_state.cx0pll.c10.pll[j];
> > >>>> +             }
> > >>>> +     }
> > >>>> +}
> > >>>> +
> > >>>> +/*
> > >>>> + * intel_dp_sanitize_seamless_boot - Snap driver state to BIOS state
> > >>> for seamless handoff.
> > >>>> + * @state: the atomic state to sanitize
> > >>>> + *
> > >>>> + * This function compares the driver's calculated new_state with the
> > >>> inherited BIOS state
> > >>>> + * (old_state). If they are within a small threshold (e.g., 0.5% for
> > >>> clock), it "snaps"
> > >>>> + * the new_state to match the BIOS state exactly. This prevents minor
> > >>> state mismatches
> > >>>> + * that would otherwise force a full modeset (and a screen flicker)
> > >>> during the initial
> > >>>> + * kernel handoff.
> > >>>> + */
> > >>>> +static void intel_dp_sanitize_seamless_boot(struct intel_atomic_state
> > >>> *state)
> > >>>> +{
> > >>>> +     struct intel_display *display = to_intel_display(state);
> > >>>> +     struct intel_crtc_state *new_crtc_state, *old_crtc_state;
> > >>>> +     struct intel_crtc *crtc;
> > >>>> +     struct intel_encoder *encoder;
> > >>>> +     int i;
> > >>>> +
> > >>>> +     for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
> > >>> new_crtc_state, i) {
> > >>>> +             /*
> > >>>> +              * We must check old_crtc_state->inherited because
> > >>> new_crtc_state->inherited
> > >>>> +              * is cleared at the start of intel_atomic_check for
> > >>> userspace commits.
> > >>>> +              */
> > >>>> +             if (!old_crtc_state->inherited ||
> > >>> !new_crtc_state->hw.active)
> > >>>> +                     continue;
> > >>>> +
> > >>>> +             if (intel_crtc_has_dp_encoder(new_crtc_state)) {
> > >>>> +                     int old_clock =
> > >>> old_crtc_state->hw.adjusted_mode.crtc_clock;
> > >>>> +                     int new_clock =
> > >>> new_crtc_state->hw.adjusted_mode.crtc_clock;
> > >>>> +                     int threshold = old_clock / 200; /* 0.5% */
> > >>>> +
> > >>>> +                     if (abs(new_clock - old_clock) <= threshold) {
> > >>>> +                             new_crtc_state->hw.pipe_mode.crtc_clock =
> > >>> old_clock;
> > >>>> +
> > >>>  new_crtc_state->hw.adjusted_mode.crtc_clock = old_clock;
> > >>>> +                             new_crtc_state->pixel_rate =
> > >>> old_crtc_state->pixel_rate;
> > >>>> +                             new_crtc_state->dp_m_n =
> > >>> old_crtc_state->dp_m_n;
> > >>>> +                     }
> > >>>> +             }
> > >>>> +
> > >>>> +             for_each_intel_encoder_mask(display->drm, encoder,
> > >>>> +                             new_crtc_state->uapi.encoder_mask) {
> > >>>> +                     if (intel_encoder_is_c10phy(encoder)) {
> > >>>> +                             if
> > >>> (!new_crtc_state->dpll_hw_state.cx0pll.ssc_enabled)
> > >>>> +
> > >>>  intel_sanitize_pll_state(old_crtc_state, new_crtc_state);
> > >>>> +                     }
> > >>>> +             }
> > >>>> +     }
> > >>>> +}
> > >>>> +
> > >>>>  /**
> > >>>>   * intel_atomic_check - validate state object
> > >>>>   * @dev: drm device
> > >>>> @@ -6447,6 +6512,8 @@ int intel_atomic_check(struct drm_device *dev,
> > >>>>       if (ret)
> > >>>>               goto fail;
> > >>>>
> > >>>> +     intel_dp_sanitize_seamless_boot(state);
> > >>>> +
> > >>>>       for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
> > >>>>               if (!intel_crtc_needs_modeset(new_crtc_state))
> > >>>>                       continue;
> > >>>
> > >>> This might fix boot state, but in a way that complicates the code
> > >>> considerably.
> > >>>
> > >>> Have you considered updating intel_pipe_config_compare instead?
> > >>>
> > >>> Kind regards,
> > >>> ~Maarten Lankhorst
> > >>>
> > > 
> 
> -- 
> Ville Syrjälä
> Intel

-- 
Ville Syrjälä
Intel