Re: [Intel-xe] [04/10] FIXME drm/i915/display: Use fence->set_deadline to boost requests

["Hogander, Jouni" <jouni.hogander@intel.com>]

Hello Maarten,

We have ongoing discussion in gfx mailing list
(https://patchwork.freedesktop.org/series/125160/). I understood we
have now there clear path forward. I will prepare one more addresing
recent comments from Ville. That patch set which will take care of
this. After getting it merged into drm-intel-next I will backport it to
drm-xe-next and it will solve this.

BR,

Jouni Högander
On Wed, 2023-10-25 at 12:20 +0200, Maarten Lankhorst wrote:
> Instead of using a i915 specific method, use the generic helper to
> combine fences and set the deadline.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  .../gpu/drm/i915/display/intel_atomic_plane.c | 47 ++++-------------
> --
>  drivers/gpu/drm/i915/display/intel_display.c  |  2 +
>  2 files changed, 11 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> index dd65498166fbf..98a59445e6f45 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> @@ -1033,7 +1033,6 @@ static int
>  intel_prepare_plane_fb(struct drm_plane *_plane,
>                        struct drm_plane_state *_new_plane_state)
>  {
> -       struct i915_sched_attr attr = { .priority =
> I915_PRIORITY_DISPLAY };
>         struct intel_plane *plane = to_intel_plane(_plane);
>         struct intel_plane_state *new_plane_state =
>                 to_intel_plane_state(_new_plane_state);
> @@ -1072,49 +1071,20 @@ intel_prepare_plane_fb(struct drm_plane
> *_plane,
>                 }
>         }
>  
> -       if (new_plane_state->uapi.fence) { /* explicit fencing */
> -               i915_gem_fence_wait_priority(new_plane_state-
> >uapi.fence,
> -                                            &attr);
> -               ret = i915_sw_fence_await_dma_fence(&state-
> >commit_ready,
> -                                                   new_plane_state-
> >uapi.fence,
> -                                                  
> i915_fence_timeout(dev_priv),
> -                                                   GFP_KERNEL);
> -               if (ret < 0)
> +       if (obj) {
> +               ret = intel_plane_pin_fb(new_plane_state);
> +               if (ret)
>                         return ret;
>         }
>  
> -       if (!obj)
> -               return 0;
> -
> -
> -       ret = intel_plane_pin_fb(new_plane_state);
> +       /* prepare after pinning */
> +       ret = drm_gem_plane_helper_prepare_fb(_plane,
> _new_plane_state);
>         if (ret)
> -               return ret;
> +               goto unpin_fb;
>  
> -       i915_gem_object_wait_priority(obj, 0, &attr);
> -
> -       if (!new_plane_state->uapi.fence) { /* implicit fencing */
> -               struct dma_resv_iter cursor;
> -               struct dma_fence *fence;
> -
> -               ret = i915_sw_fence_await_reservation(&state-
> >commit_ready,
> -                                                     obj->base.resv,
> false,
> -                                                    
> i915_fence_timeout(dev_priv),
> -                                                     GFP_KERNEL);
> -               if (ret < 0)
> -                       goto unpin_fb;
> -
> -               dma_resv_iter_begin(&cursor, obj->base.resv,
> -                                   DMA_RESV_USAGE_WRITE);
> -               dma_resv_for_each_fence_unlocked(&cursor, fence) {
> -
>                        intel_display_rps_boost_after_vblank(new_plane_
> state->hw.crtc,
> -                                                            fence);
> -               }
> -               dma_resv_iter_end(&cursor);
> -       } else {
> +       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
> @@ -1124,7 +1094,8 @@ intel_prepare_plane_fb(struct drm_plane
> *_plane,
>          * that are not quite steady state without resorting to
> forcing
>          * maximum clocks following a vblank miss (see
> do_rps_boost()).
>          */
> -       intel_display_rps_mark_interactive(dev_priv, state, true);
> +       if (obj)
> +               intel_display_rps_mark_interactive(dev_priv, state,
> true);
>  
>         return 0;
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index 478b8764101a2..1b66d5adcebee 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -6599,6 +6599,8 @@ static int intel_atomic_prepare_commit(struct
> intel_atomic_state *state)
>         if (ret < 0)
>                 return ret;
>  
> +       drm_atomic_helper_set_fence_deadline(state->base.dev, &state-
> >base);
> +
>         for_each_new_intel_crtc_in_state(state, crtc, crtc_state, i)
> {
>                 if (intel_crtc_needs_color_update(crtc_state))
>                         intel_color_prepare_commit(crtc_state);