From: Rodrigo Vivi <rodrigo.vivi@intel.com>
To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: intel-xe@lists.freedesktop.org
Subject: Re: [Intel-xe] [PATCH 3/3] drm/i915: Block in cleanup for legacy cursor updates
Date: Wed, 20 Sep 2023 16:06:34 -0400 [thread overview]
Message-ID: <ZQtQyjMZmtzuVmKM@intel.com> (raw)
In-Reply-To: <20230915173646.116724-3-maarten.lankhorst@linux.intel.com>
On Fri, Sep 15, 2023 at 07:36:46PM +0200, Maarten Lankhorst wrote:
we need a commit message with justification for those. and probably
better to land them first to i915 and then port to xe like Jouni
is doing.
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
> .../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 | 36 +++++++++++++++++--
> drivers/gpu/drm/i915/display/intel_crtc.h | 6 ++--
> drivers/gpu/drm/i915/display/intel_display.c | 4 +--
> .../drm/i915/display/intel_display_types.h | 5 +++
> 6 files changed, 65 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> index cb60165bc4156..ba9e6c7809bee 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> @@ -1167,6 +1167,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);
> }
>
> @@ -1179,3 +1182,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)
> +{ }
> +
> +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 f06b987f55582..e3b8ce6072284 100644
> --- a/drivers/gpu/drm/i915/display/intel_crtc.c
> +++ b/drivers/gpu/drm/i915/display/intel_crtc.c
> @@ -470,6 +470,7 @@ static int intel_mode_vblank_start(const struct drm_display_mode *mode)
>
> /**
> * intel_pipe_update_start() - start update of a set of display registers
> + * @state: the intel atomic state
> * @new_crtc_state: the new crtc state
> *
> * Mark the start of an update to pipe registers that should be updated
> @@ -480,7 +481,8 @@ static int intel_mode_vblank_start(const struct drm_display_mode *mode)
> * until a subsequent call to intel_pipe_update_end(). That is done to
> * avoid random delays.
> */
> -void intel_pipe_update_start(struct intel_crtc_state *new_crtc_state)
> +void intel_pipe_update_start(struct intel_atomic_state *state,
> + struct intel_crtc_state *new_crtc_state)
> {
> struct intel_crtc *crtc = to_intel_crtc(new_crtc_state->uapi.crtc);
> struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> @@ -500,6 +502,19 @@ void intel_pipe_update_start(struct intel_crtc_state *new_crtc_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);
> + }
> + }
> +
> if (new_crtc_state->vrr.enable) {
> if (intel_vrr_is_push_sent(new_crtc_state))
> vblank_start = intel_vrr_vmin_vblank_start(new_crtc_state);
> @@ -635,13 +650,15 @@ static void dbg_vblank_evade(struct intel_crtc *crtc, ktime_t end) {}
>
> /**
> * intel_pipe_update_end() - end update of a set of display registers
> + * @state: the intel atomic state
> * @new_crtc_state: the new crtc state
> *
> * Mark the end of an update started with intel_pipe_update_start(). This
> * re-enables interrupts and verifies the update was actually completed
> * before a vblank.
> */
> -void intel_pipe_update_end(struct intel_crtc_state *new_crtc_state)
> +void intel_pipe_update_end(struct intel_atomic_state *state,
> + struct intel_crtc_state *new_crtc_state)
> {
> struct intel_crtc *crtc = to_intel_crtc(new_crtc_state->uapi.crtc);
> enum pipe pipe = crtc->pipe;
> @@ -687,6 +704,21 @@ void intel_pipe_update_end(struct intel_crtc_state *new_crtc_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
> diff --git a/drivers/gpu/drm/i915/display/intel_crtc.h b/drivers/gpu/drm/i915/display/intel_crtc.h
> index 51a4c8df9e657..ca7f45a454a0a 100644
> --- a/drivers/gpu/drm/i915/display/intel_crtc.h
> +++ b/drivers/gpu/drm/i915/display/intel_crtc.h
> @@ -36,8 +36,10 @@ void intel_crtc_state_reset(struct intel_crtc_state *crtc_state,
> u32 intel_crtc_get_vblank_counter(struct intel_crtc *crtc);
> void intel_crtc_vblank_on(const struct intel_crtc_state *crtc_state);
> void intel_crtc_vblank_off(const struct intel_crtc_state *crtc_state);
> -void intel_pipe_update_start(struct intel_crtc_state *new_crtc_state);
> -void intel_pipe_update_end(struct intel_crtc_state *new_crtc_state);
> +void intel_pipe_update_start(struct intel_atomic_state *state,
> + struct intel_crtc_state *new_crtc_state);
> +void intel_pipe_update_end(struct intel_atomic_state *state,
> + struct intel_crtc_state *new_crtc_state);
> void intel_wait_for_vblank_workers(struct intel_atomic_state *state);
> struct intel_crtc *intel_first_crtc(struct drm_i915_private *i915);
> struct intel_crtc *intel_crtc_for_pipe(struct drm_i915_private *i915,
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 9df8081f78d97..ce493ebbd9b93 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -6610,7 +6610,7 @@ static void intel_update_crtc(struct intel_atomic_state *state,
> intel_crtc_planes_update_noarm(state, crtc);
>
> /* Perform vblank evasion around commit operation */
> - intel_pipe_update_start(new_crtc_state);
> + intel_pipe_update_start(state, new_crtc_state);
>
> commit_pipe_pre_planes(state, crtc);
>
> @@ -6618,7 +6618,7 @@ static void intel_update_crtc(struct intel_atomic_state *state,
>
> commit_pipe_post_planes(state, crtc);
>
> - intel_pipe_update_end(new_crtc_state);
> + intel_pipe_update_end(state, new_crtc_state);
>
> /*
> * We usually enable FIFO underrun interrupts as part of the
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index c0931d89d0dd9..20620ae074069 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -769,6 +769,11 @@ struct intel_plane_state {
>
> struct drm_rect psr2_sel_fetch_area;
>
> + /*
> + * Unpin work for cursor fb updates.
> + */
> + struct drm_vblank_work unpin_work;
> +
> /* Clear Color Value */
> u64 ccval;
>
> --
> 2.39.2
>
next prev parent reply other threads:[~2023-09-20 20:06 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-15 17:36 [Intel-xe] [PATCH 1/3] fixup! drm/xe/display: Implement display support Maarten Lankhorst
2023-09-15 17:36 ` [Intel-xe] [PATCH 2/3] drm/i915: Swap ggtt_vma during legacy cursor update Maarten Lankhorst
2023-09-15 17:36 ` [Intel-xe] [PATCH 3/3] drm/i915: Block in cleanup for legacy cursor updates Maarten Lankhorst
2023-09-20 20:06 ` Rodrigo Vivi [this message]
2023-09-15 18:13 ` [Intel-xe] ✓ CI.Patch_applied: success for series starting with [1/3] fixup! drm/xe/display: Implement display support Patchwork
2023-09-15 18:14 ` [Intel-xe] ✗ CI.checkpatch: warning " Patchwork
2023-09-15 18:15 ` [Intel-xe] ✓ CI.KUnit: success " Patchwork
2023-09-15 18:22 ` [Intel-xe] ✓ CI.Build: " Patchwork
2023-09-15 18:22 ` [Intel-xe] ✗ CI.Hooks: failure " Patchwork
2023-09-15 18:23 ` [Intel-xe] ✗ CI.checksparse: warning " Patchwork
2023-09-15 18:55 ` [Intel-xe] ✓ CI.BAT: success " Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZQtQyjMZmtzuVmKM@intel.com \
--to=rodrigo.vivi@intel.com \
--cc=intel-xe@lists.freedesktop.org \
--cc=maarten.lankhorst@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.