Re: [PATCH 05/10] drm/i915: Move initial plane vblank wait into display code

[Jani Nikula <jani.nikula@linux.intel.com>]

On Fri, 10 Apr 2026, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> The initial plane vblank wait operates on display registers,
> so it really belongs in the display code proper. Move it there.
>
> We can use intel_parent_irq_enabled() to determine if we can
> rely on interrupts or not.
>
> On average we should end up waiting half a frame here, so the
> polling interval can be fairly long. 1 ms (which actually
> makes poll_timeout_us() use ~250-1000 usec) seems good enough
> to me.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Yes, it's all nicer this way. Just goes on to show how it's sometimes
better to do things one way to get things rolling, and only afterwards
you understand how it should be done.

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> ---
>  .../drm/i915/display/intel_initial_plane.c    | 22 ++++++++++++++++++-
>  drivers/gpu/drm/i915/i915_initial_plane.c     |  6 -----
>  drivers/gpu/drm/xe/display/xe_initial_plane.c | 19 ----------------
>  include/drm/intel/display_parent_interface.h  |  2 --
>  4 files changed, 21 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_initial_plane.c b/drivers/gpu/drm/i915/display/intel_initial_plane.c
> index 911d67dceba9..74e10d34c63c 100644
> --- a/drivers/gpu/drm/i915/display/intel_initial_plane.c
> +++ b/drivers/gpu/drm/i915/display/intel_initial_plane.c
> @@ -1,14 +1,20 @@
>  // SPDX-License-Identifier: MIT
>  /* Copyright © 2025 Intel Corporation */
>  
> +#include <linux/iopoll.h>
> +
>  #include <drm/drm_print.h>
>  #include <drm/intel/display_parent_interface.h>
>  
> +#include "intel_crtc.h"
> +#include "intel_de.h"
>  #include "intel_display_core.h"
> +#include "intel_display_regs.h"
>  #include "intel_display_types.h"
>  #include "intel_fb.h"
>  #include "intel_frontbuffer.h"
>  #include "intel_initial_plane.h"
> +#include "intel_parent.h"
>  #include "intel_plane.h"
>  
>  struct intel_initial_plane_configs {
> @@ -18,8 +24,22 @@ struct intel_initial_plane_configs {
>  void intel_initial_plane_vblank_wait(struct intel_crtc *crtc)
>  {
>  	struct intel_display *display = to_intel_display(crtc);
> +	u32 start_ts, end_ts;
> +	int ret;
>  
> -	display->parent->initial_plane->vblank_wait(&crtc->base);
> +	/* xe doesn't have interrupts enabled this early */
> +	if (intel_parent_irq_enabled(display)) {
> +		intel_crtc_wait_for_next_vblank(crtc);
> +		return;
> +	}
> +
> +	start_ts = intel_de_read(display, PIPE_FRMTMSTMP(crtc->pipe));
> +
> +	ret = poll_timeout_us(end_ts = intel_de_read(display, PIPE_FRMTMSTMP(crtc->pipe)),
> +			      end_ts != start_ts, 1000, 40 * 1000, false);
> +	if (ret)
> +		drm_warn(display->drm, "[CRTC:%d:%s] early vblank wait timed out\n",
> +			 crtc->base.base.id, crtc->base.name);
>  }
>  
>  static const struct intel_plane_state *
> diff --git a/drivers/gpu/drm/i915/i915_initial_plane.c b/drivers/gpu/drm/i915/i915_initial_plane.c
> index 5cb1adde67b6..7775e657271b 100644
> --- a/drivers/gpu/drm/i915/i915_initial_plane.c
> +++ b/drivers/gpu/drm/i915/i915_initial_plane.c
> @@ -16,11 +16,6 @@
>  #include "i915_drv.h"
>  #include "i915_initial_plane.h"
>  
> -static void i915_initial_plane_vblank_wait(struct drm_crtc *crtc)
> -{
> -	intel_crtc_wait_for_next_vblank(to_intel_crtc(crtc));
> -}
> -
>  static enum intel_memory_type
>  initial_plane_memory_type(struct drm_i915_private *i915)
>  {
> @@ -282,7 +277,6 @@ static void i915_plane_config_fini(struct intel_initial_plane_config *plane_conf
>  }
>  
>  const struct intel_display_initial_plane_interface i915_display_initial_plane_interface = {
> -	.vblank_wait = i915_initial_plane_vblank_wait,
>  	.alloc_obj = i915_alloc_initial_plane_obj,
>  	.setup = i915_initial_plane_setup,
>  	.config_fini = i915_plane_config_fini,
> diff --git a/drivers/gpu/drm/xe/display/xe_initial_plane.c b/drivers/gpu/drm/xe/display/xe_initial_plane.c
> index 8f2d0244c03f..37bd15d12169 100644
> --- a/drivers/gpu/drm/xe/display/xe_initial_plane.c
> +++ b/drivers/gpu/drm/xe/display/xe_initial_plane.c
> @@ -7,8 +7,6 @@
>  
>  #include "regs/xe_gtt_defs.h"
>  
> -#include "intel_crtc.h"
> -#include "intel_display_regs.h"
>  #include "intel_display_types.h"
>  #include "intel_fb.h"
>  #include "intel_fb_pin.h"
> @@ -19,22 +17,6 @@
>  #include "xe_mmio.h"
>  #include "xe_vram_types.h"
>  
> -/* Early xe has no irq */
> -static void xe_initial_plane_vblank_wait(struct drm_crtc *_crtc)
> -{
> -	struct intel_crtc *crtc = to_intel_crtc(_crtc);
> -	struct xe_device *xe = to_xe_device(crtc->base.dev);
> -	struct xe_reg pipe_frmtmstmp = XE_REG(i915_mmio_reg_offset(PIPE_FRMTMSTMP(crtc->pipe)));
> -	u32 timestamp;
> -	int ret;
> -
> -	timestamp = xe_mmio_read32(xe_root_tile_mmio(xe), pipe_frmtmstmp);
> -
> -	ret = xe_mmio_wait32_not(xe_root_tile_mmio(xe), pipe_frmtmstmp, ~0U, timestamp, 40000U, &timestamp, false);
> -	if (ret < 0)
> -		drm_warn(&xe->drm, "waiting for early vblank failed with %i\n", ret);
> -}
> -
>  static struct xe_bo *
>  initial_plane_bo(struct xe_device *xe,
>  		 struct intel_initial_plane_config *plane_config)
> @@ -172,7 +154,6 @@ static void xe_plane_config_fini(struct intel_initial_plane_config *plane_config
>  }
>  
>  const struct intel_display_initial_plane_interface xe_display_initial_plane_interface = {
> -	.vblank_wait = xe_initial_plane_vblank_wait,
>  	.alloc_obj = xe_alloc_initial_plane_obj,
>  	.setup = xe_initial_plane_setup,
>  	.config_fini = xe_plane_config_fini,
> diff --git a/include/drm/intel/display_parent_interface.h b/include/drm/intel/display_parent_interface.h
> index 9041897c772e..b513e3f9924d 100644
> --- a/include/drm/intel/display_parent_interface.h
> +++ b/include/drm/intel/display_parent_interface.h
> @@ -8,7 +8,6 @@
>  
>  enum vlv_iosf_sb_unit;
>  struct dma_fence;
> -struct drm_crtc;
>  struct drm_device;
>  struct drm_file;
>  struct drm_framebuffer;
> @@ -87,7 +86,6 @@ struct intel_display_hdcp_interface {
>  };
>  
>  struct intel_display_initial_plane_interface {
> -	void (*vblank_wait)(struct drm_crtc *crtc);
>  	struct drm_gem_object *(*alloc_obj)(struct drm_device *drm, struct intel_initial_plane_config *plane_config);
>  	int (*setup)(struct drm_plane_state *plane_state, struct intel_initial_plane_config *plane_config,
>  		     struct drm_framebuffer *fb, struct i915_vma *vma);

-- 
Jani Nikula, Intel