Re: [PATCH 12/14] drm/xe/display: Use async flip for flipping initial fb.

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

On Fri, Sep 27, 2024 at 04:14:02PM +0200, Maarten Lankhorst wrote:
> Since memirq requires a ggtt allocation, irqs will be initialised
> later. This means we cannot rely on drm_wait_for_vblank any more as
> it requires interrupts.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> 
> TODO: Check for flip completion?
> ---
>  drivers/gpu/drm/xe/display/xe_plane_initial.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/display/xe_plane_initial.c b/drivers/gpu/drm/xe/display/xe_plane_initial.c
> index 4660db0aecb6..bad074540b55 100644
> --- a/drivers/gpu/drm/xe/display/xe_plane_initial.c
> +++ b/drivers/gpu/drm/xe/display/xe_plane_initial.c
> @@ -17,6 +17,8 @@
>  #include "intel_fb_pin.h"
>  #include "intel_frontbuffer.h"
>  #include "intel_plane_initial.h"
> +#include "intel_uncore.h"
> +#include "skl_universal_plane_regs.h"
>  #include "xe_bo.h"
>  #include "xe_wa.h"
>  
> @@ -187,6 +189,7 @@ static void
>  intel_find_initial_plane_obj(struct intel_crtc *crtc,
>  			     struct intel_initial_plane_config plane_configs[])
>  {
> +	struct xe_device *xe = to_xe_device(crtc->base.dev);
>  	struct intel_initial_plane_config *plane_config =
>  		&plane_configs[crtc->pipe];
>  	struct intel_plane *plane =
> @@ -235,6 +238,10 @@ intel_find_initial_plane_obj(struct intel_crtc *crtc,
>  	plane_state->uapi.crtc = &crtc->base;
>  	intel_plane_copy_uapi_to_hw_state(plane_state, plane_state, crtc);
>  
> +	/* Flip async, we don't have interrupts yet */
> +	plane_state->ctl = intel_uncore_read(&xe->uncore, PLANE_CTL(plane->pipe, PLANE_1));
> +	plane->async_flip(plane, to_intel_crtc_state(crtc->base.state), plane_state, true);
> +
>  	atomic_or(plane->frontbuffer_bit, &to_intel_frontbuffer(fb)->bits);
>  
>  	plane_config->vma = vma;
> @@ -292,9 +299,6 @@ void intel_initial_plane_config(struct drm_i915_private *i915)
>  		 */
>  		intel_find_initial_plane_obj(crtc, plane_configs);
>  
> -		if (i915->display.funcs.display->fixup_initial_plane_config(crtc, plane_config))
> -			intel_crtc_wait_for_next_vblank(crtc);

Argh. I think the best solution would probably be to provide
an intel_crtc_wait_for_next_vblank_early()/etc. that just
falls back to polling the status bit if interrupts aren't
available.

Or we could also just poll the live surface register
instead (though that is not available on ancient hardware
so would have to assert that it's not used there).

I suppose another option might be to just usleep() in
the fallback. This would have the benefit of not needing
any hardware specific implementations. This doesn't really
need to be that accurate as long it guarantees that we
wait for at least one frame duration.

> -
>  		plane_config_fini(plane_config);
>  	}
>  }
> -- 
> 2.45.2

-- 
Ville Syrjälä
Intel