> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of Maarten
> Lankhorst
> Sent: Thursday, November 9, 2023 9:05 PM
> To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> Subject: [PATCH 3/3] drm/i915: Use a different vblank worker for
> atomic unpin
>
> For the atomic codepath we unpin_work in old_plane_state to unpin the old fb. As
> this happened after swapping state, this is allowed.
>
> Use the unpin_work only as a barrier, and keep doing the actual unpinning in the
> atomic path.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
> ---
> .../gpu/drm/i915/display/intel_atomic_plane.c | 18 ++++++++++++
> .../gpu/drm/i915/display/intel_atomic_plane.h | 2 ++
> drivers/gpu/drm/i915/display/intel_crtc.c | 28 +++++++++++++++++++
> 3 files changed, 48 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> index 06c2455bdd788..6ddb4f4ec79ac 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> @@ -1164,6 +1164,9 @@ intel_cleanup_plane_fb(struct drm_plane *plane,
> intel_display_rps_mark_interactive(dev_priv, state, false);
>
> /* Should only be called after a successful intel_prepare_plane_fb()! */
> + if (old_plane_state->unpin_work.vblank)
> + drm_vblank_work_flush(&old_plane_state->unpin_work);
> +
> intel_plane_unpin_fb(old_plane_state);
> }
>
> @@ -1176,3 +1179,18 @@ void intel_plane_helper_add(struct intel_plane
> *plane) {
> drm_plane_helper_add(&plane->base, &intel_plane_helper_funcs); }
> +
> +/* Completion is enough */
> +static void intel_plane_cursor_vblank_work(struct kthread_work *base) {
> +}
Not quite clear the role of this worker here. Can you elaborate a bit more.
Once we set a work and schedule it, what is the expectation from the worker here.
Regards,
Uma Shankar
> +
> +void intel_plane_init_cursor_vblank_work(struct intel_plane_state
> *old_plane_state,
> + struct intel_plane_state
> *new_plane_state) {
> + if (!old_plane_state->ggtt_vma ||
> + old_plane_state->ggtt_vma == new_plane_state->ggtt_vma)
> + return;
> +
> + drm_vblank_work_init(&old_plane_state->unpin_work, old_plane_state-
> >uapi.crtc,
> + intel_plane_cursor_vblank_work); }
> diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.h
> b/drivers/gpu/drm/i915/display/intel_atomic_plane.h
> index 191dad0efc8e6..5a897cf6fa021 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.h
> +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.h
> @@ -66,5 +66,7 @@ int intel_plane_check_src_coordinates(struct
> intel_plane_state *plane_state); void intel_plane_set_invisible(struct
> intel_crtc_state *crtc_state,
> struct intel_plane_state *plane_state); void
> intel_plane_helper_add(struct intel_plane *plane);
> +void intel_plane_init_cursor_vblank_work(struct intel_plane_state
> *old_plane_state,
> + struct intel_plane_state
> *new_plane_state);
>
> #endif /* __INTEL_ATOMIC_PLANE_H__ */
> diff --git a/drivers/gpu/drm/i915/display/intel_crtc.c
> b/drivers/gpu/drm/i915/display/intel_crtc.c
> index 1fd068e6e26ca..755c40fd0ac13 100644
> --- a/drivers/gpu/drm/i915/display/intel_crtc.c
> +++ b/drivers/gpu/drm/i915/display/intel_crtc.c
> @@ -559,6 +559,19 @@ void intel_pipe_update_start(struct intel_atomic_state
> *state,
> if (intel_crtc_needs_vblank_work(new_crtc_state))
> intel_crtc_vblank_work_init(new_crtc_state);
>
> + if (state->base.legacy_cursor_update) {
> + struct intel_plane *plane;
> + struct intel_plane_state *old_plane_state, *new_plane_state;
> + int i;
> +
> + for_each_oldnew_intel_plane_in_state(state, plane,
> old_plane_state,
> + new_plane_state, i) {
> + if (old_plane_state->uapi.crtc == &crtc->base)
> +
> intel_plane_init_cursor_vblank_work(old_plane_state,
> +
> new_plane_state);
> + }
> + }
> +
> intel_crtc_vblank_evade_scanlines(state, crtc, &min, &max,
> &vblank_start);
> if (min <= 0 || max <= 0)
> goto irq_disable;
> @@ -721,6 +734,21 @@ void intel_pipe_update_end(struct intel_atomic_state
> *state,
> new_crtc_state->uapi.event = NULL;
> }
>
> + if (state->base.legacy_cursor_update) {
> + struct intel_plane *plane;
> + struct intel_plane_state *old_plane_state;
> + int i;
> +
> + for_each_old_intel_plane_in_state(state, plane,
> old_plane_state, i) {
> + if (old_plane_state->uapi.crtc == &crtc->base &&
> + old_plane_state->unpin_work.vblank) {
> + drm_vblank_work_schedule(&old_plane_state-
> >unpin_work,
> +
> drm_crtc_accurate_vblank_count(&crtc->base) + 1,
> + false);
> + }
> + }
> + }
> +
> /*
> * Send VRR Push to terminate Vblank. If we are already in vblank
> * this has to be done _after_ sampling the frame counter, as
> --
> 2.39.2
