Re: [PATCH 4/5] drm/i915/rps: make fence priority setting part of the rps interface

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Thu, Nov 06, 2025 at 04:43:12PM +0200, Jani Nikula wrote:
> This is perhaps not ideal, but simplifies the interfaces, and allows us
> to get rid of the compat header in xe.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display_rps.c    |  2 ++
>  drivers/gpu/drm/i915/display/intel_plane.c          |  6 +-----
>  drivers/gpu/drm/i915/gt/intel_rps.c                 |  9 +++++++++
>  .../xe/compat-i915-headers/gem/i915_gem_object.h    | 13 -------------
>  include/drm/intel/display_parent_interface.h        |  1 +
>  5 files changed, 13 insertions(+), 18 deletions(-)
>  delete mode 100644 drivers/gpu/drm/xe/compat-i915-headers/gem/i915_gem_object.h
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_rps.c b/drivers/gpu/drm/i915/display/intel_display_rps.c
> index 44cb9dba0c19..a2d57671c419 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_rps.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_rps.c
> @@ -48,6 +48,8 @@ void intel_display_rps_boost_after_vblank(struct drm_crtc *crtc,
>  	if (!display->parent->rps)
>  		return;
>  
> +	display->parent->rps->priority_display(fence);

This is quite confusing now. This thing is about the scheduler
and nothing to do with RPS boosting.

If we were to mix these into one interface somehow then I don't think
it should be called rps_something.

As a side note, there's also some kind of performance problem with this
thing when using GUC submission. I first noticed the problem on DG2
where GPU utilization was very low on simple workloads, and skipping
this priority shuffling increases the performance considerably. Though
even with this removed, GUC submissions is still very slow compared to
execlists (when compared on TGL which still supports both submission
paths). I haven't really had time to look into it in any great detail,
but we might need to get rid of this thing for the GUC submission path.
Though I suppose the interface changes don't really matter for that
because the submission path checks would be on the i915 side anyway.

> +
>  	if (DISPLAY_VER(display) < 6)
>  		return;
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_plane.c b/drivers/gpu/drm/i915/display/intel_plane.c
> index 505c776c0585..28ee9502b596 100644
> --- a/drivers/gpu/drm/i915/display/intel_plane.c
> +++ b/drivers/gpu/drm/i915/display/intel_plane.c
> @@ -45,7 +45,6 @@
>  #include <drm/drm_panic.h>
>  #include <drm/drm_print.h>
>  
> -#include "gem/i915_gem_object.h"
>  #include "i9xx_plane_regs.h"
>  #include "intel_cdclk.h"
>  #include "intel_cursor.h"
> @@ -1176,12 +1175,9 @@ intel_prepare_plane_fb(struct drm_plane *_plane,
>  	if (ret < 0)
>  		goto unpin_fb;
>  
> -	if (new_plane_state->uapi.fence) {
> -		i915_gem_fence_wait_priority_display(new_plane_state->uapi.fence);
> -
> +	if (new_plane_state->uapi.fence)
>  		intel_display_rps_boost_after_vblank(new_plane_state->hw.crtc,
>  						     new_plane_state->uapi.fence);
> -	}
>  
>  	/*
>  	 * We declare pageflips to be interactive and so merit a small bias
> diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c b/drivers/gpu/drm/i915/gt/intel_rps.c
> index 05b21de6c24b..1af39198e0c5 100644
> --- a/drivers/gpu/drm/i915/gt/intel_rps.c
> +++ b/drivers/gpu/drm/i915/gt/intel_rps.c
> @@ -2915,6 +2915,14 @@ bool i915_gpu_turbo_disable(void)
>  }
>  EXPORT_SYMBOL_GPL(i915_gpu_turbo_disable);
>  
> +static void priority_display(struct dma_fence *fence)
> +{
> +	if (!dma_fence_is_i915(fence))
> +		return;
> +
> +	i915_gem_fence_wait_priority_display(fence);
> +}
> +
>  static void boost(struct dma_fence *fence)
>  {
>  	struct i915_request *rq;
> @@ -2948,6 +2956,7 @@ static void ilk_irq_handler(struct drm_device *drm)
>  }
>  
>  const struct intel_display_rps_interface i915_display_rps_interface = {
> +	.priority_display = priority_display,
>  	.boost = boost,
>  	.mark_interactive = mark_interactive,
>  	.ilk_irq_handler = ilk_irq_handler,
> diff --git a/drivers/gpu/drm/xe/compat-i915-headers/gem/i915_gem_object.h b/drivers/gpu/drm/xe/compat-i915-headers/gem/i915_gem_object.h
> deleted file mode 100644
> index 0548b2e0316f..000000000000
> --- a/drivers/gpu/drm/xe/compat-i915-headers/gem/i915_gem_object.h
> +++ /dev/null
> @@ -1,13 +0,0 @@
> -/* SPDX-License-Identifier: MIT */
> -/* Copyright © 2025 Intel Corporation */
> -
> -#ifndef __I915_GEM_OBJECT_H__
> -#define __I915_GEM_OBJECT_H__
> -
> -struct dma_fence;
> -
> -static inline void i915_gem_fence_wait_priority_display(struct dma_fence *fence)
> -{
> -}
> -
> -#endif
> diff --git a/include/drm/intel/display_parent_interface.h b/include/drm/intel/display_parent_interface.h
> index 8920404545be..7614b35660c9 100644
> --- a/include/drm/intel/display_parent_interface.h
> +++ b/include/drm/intel/display_parent_interface.h
> @@ -27,6 +27,7 @@ struct intel_display_rpm_interface {
>  };
>  
>  struct intel_display_rps_interface {
> +	void (*priority_display)(struct dma_fence *fence);
>  	void (*boost)(struct dma_fence *fence);
>  	void (*mark_interactive)(struct drm_device *drm, bool interactive);
>  	void (*ilk_irq_handler)(struct drm_device *drm);
> -- 
> 2.47.3

-- 
Ville Syrjälä
Intel