Skip to content

Commit 5c8107d

Browse files
vsyrjalajlahtine-intel
authored andcommitted
drm/i915: Fix the async flip wm0/ddb optimization
The current implementation of the async flip wm0/ddb optimization does not work at all. The biggest problem is that we skip the whole intel_pipe_update_{start,end}() dance and thus never actually complete the commit that is trying to do the wm/ddb change. To fix this we need to move the do_async_flip flag to the crtc state since we handle commits per-pipe, not per-plane. Also since all planes can now be included in the first/last "async flip" (which gets converted to a sync flip to do the wm/ddb mangling) we need to be more careful when checking if the plane state is async flip comptatible. Only planes doing the async flip should be checked and other planes are perfectly fine not adhereing to any async flip related limitations. However for subsequent commits which are actually going do the async flip in hardware we want to make sure no other planes are in the state. That should never happen assuming we did our job correctly, so we'll toss in a WARN to make sure we catch any bugs here. Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> Fixes: c3639f3 ("drm/i915: Use wm0 only during async flips for DG2") Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20220214105532.13049-4-ville.syrjala@linux.intel.com Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> (cherry picked from commit 2e08437) Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
1 parent 176c0b5 commit 5c8107d

5 files changed

Lines changed: 31 additions & 25 deletions

File tree

drivers/gpu/drm/i915/display/intel_atomic.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,7 @@ intel_crtc_duplicate_state(struct drm_crtc *crtc)
262262
crtc_state->preload_luts = false;
263263
crtc_state->inherited = false;
264264
crtc_state->wm.need_postvbl_update = false;
265+
crtc_state->do_async_flip = false;
265266
crtc_state->fb_bits = 0;
266267
crtc_state->update_planes = 0;
267268
crtc_state->dsb = NULL;

drivers/gpu/drm/i915/display/intel_atomic_plane.c

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,6 @@ intel_plane_duplicate_state(struct drm_plane *plane)
110110
intel_state->ggtt_vma = NULL;
111111
intel_state->dpt_vma = NULL;
112112
intel_state->flags = 0;
113-
intel_state->do_async_flip = false;
114113

115114
/* add reference to fb */
116115
if (intel_state->hw.fb)
@@ -506,7 +505,7 @@ static int intel_plane_atomic_calc_changes(const struct intel_crtc_state *old_cr
506505
new_crtc_state->disable_lp_wm = true;
507506

508507
if (intel_plane_do_async_flip(plane, old_crtc_state, new_crtc_state))
509-
new_plane_state->do_async_flip = true;
508+
new_crtc_state->do_async_flip = true;
510509

511510
return 0;
512511
}
@@ -678,7 +677,7 @@ void intel_plane_update_arm(struct intel_plane *plane,
678677

679678
trace_intel_plane_update_arm(&plane->base, crtc);
680679

681-
if (plane_state->do_async_flip)
680+
if (crtc_state->do_async_flip && plane->async_flip)
682681
plane->async_flip(plane, crtc_state, plane_state, true);
683682
else
684683
plane->update_arm(plane, crtc_state, plane_state);
@@ -703,7 +702,7 @@ void intel_crtc_planes_update_noarm(struct intel_atomic_state *state,
703702
struct intel_plane *plane;
704703
int i;
705704

706-
if (new_crtc_state->uapi.async_flip)
705+
if (new_crtc_state->do_async_flip)
707706
return;
708707

709708
/*

drivers/gpu/drm/i915/display/intel_crtc.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -485,7 +485,7 @@ void intel_pipe_update_start(struct intel_crtc_state *new_crtc_state)
485485
intel_crtc_has_type(new_crtc_state, INTEL_OUTPUT_DSI);
486486
DEFINE_WAIT(wait);
487487

488-
if (new_crtc_state->uapi.async_flip)
488+
if (new_crtc_state->do_async_flip)
489489
return;
490490

491491
if (intel_crtc_needs_vblank_work(new_crtc_state))
@@ -630,7 +630,7 @@ void intel_pipe_update_end(struct intel_crtc_state *new_crtc_state)
630630
ktime_t end_vbl_time = ktime_get();
631631
struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
632632

633-
if (new_crtc_state->uapi.async_flip)
633+
if (new_crtc_state->do_async_flip)
634634
return;
635635

636636
trace_intel_pipe_update_end(crtc, end_vbl_count, scanline_end);

drivers/gpu/drm/i915/display/intel_display.c

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1263,10 +1263,8 @@ static void intel_crtc_enable_flip_done(struct intel_atomic_state *state,
12631263
int i;
12641264

12651265
for_each_new_intel_plane_in_state(state, plane, plane_state, i) {
1266-
if (plane->enable_flip_done &&
1267-
plane->pipe == crtc->pipe &&
1268-
update_planes & BIT(plane->id) &&
1269-
plane_state->do_async_flip)
1266+
if (plane->pipe == crtc->pipe &&
1267+
update_planes & BIT(plane->id))
12701268
plane->enable_flip_done(plane);
12711269
}
12721270
}
@@ -1282,10 +1280,8 @@ static void intel_crtc_disable_flip_done(struct intel_atomic_state *state,
12821280
int i;
12831281

12841282
for_each_new_intel_plane_in_state(state, plane, plane_state, i) {
1285-
if (plane->disable_flip_done &&
1286-
plane->pipe == crtc->pipe &&
1287-
update_planes & BIT(plane->id) &&
1288-
plane_state->do_async_flip)
1283+
if (plane->pipe == crtc->pipe &&
1284+
update_planes & BIT(plane->id))
12891285
plane->disable_flip_done(plane);
12901286
}
12911287
}
@@ -7504,15 +7500,25 @@ static int intel_async_flip_check_hw(struct intel_atomic_state *state, struct in
75047500
continue;
75057501

75067502
/*
7507-
* TODO: Async flip is only supported through the page flip IOCTL
7508-
* as of now. So support currently added for primary plane only.
7509-
* Support for other planes on platforms on which supports
7510-
* this(vlv/chv and icl+) should be added when async flip is
7511-
* enabled in the atomic IOCTL path.
7503+
* Only async flip capable planes should be in the state
7504+
* if we're really about to ask the hardware to perform
7505+
* an async flip. We should never get this far otherwise.
75127506
*/
7513-
if (!plane->async_flip)
7507+
if (drm_WARN_ON(&i915->drm,
7508+
new_crtc_state->do_async_flip && !plane->async_flip))
75147509
return -EINVAL;
75157510

7511+
/*
7512+
* Only check async flip capable planes other planes
7513+
* may be involved in the initial commit due to
7514+
* the wm0/ddb optimization.
7515+
*
7516+
* TODO maybe should track which planes actually
7517+
* were requested to do the async flip...
7518+
*/
7519+
if (!plane->async_flip)
7520+
continue;
7521+
75167522
/*
75177523
* FIXME: This check is kept generic for all platforms.
75187524
* Need to verify this for all gen9 platforms to enable
@@ -8463,7 +8469,7 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
84638469
intel_dbuf_pre_plane_update(state);
84648470

84658471
for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
8466-
if (new_crtc_state->uapi.async_flip)
8472+
if (new_crtc_state->do_async_flip)
84678473
intel_crtc_enable_flip_done(state, crtc);
84688474
}
84698475

@@ -8489,7 +8495,7 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
84898495
drm_atomic_helper_wait_for_flip_done(dev, &state->base);
84908496

84918497
for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
8492-
if (new_crtc_state->uapi.async_flip)
8498+
if (new_crtc_state->do_async_flip)
84938499
intel_crtc_disable_flip_done(state, crtc);
84948500
}
84958501

drivers/gpu/drm/i915/display/intel_display_types.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -613,9 +613,6 @@ struct intel_plane_state {
613613

614614
struct intel_fb_view view;
615615

616-
/* Indicates if async flip is required */
617-
bool do_async_flip;
618-
619616
/* Plane pxp decryption state */
620617
bool decrypt;
621618

@@ -951,6 +948,9 @@ struct intel_crtc_state {
951948
bool preload_luts;
952949
bool inherited; /* state inherited from BIOS? */
953950

951+
/* Ask the hardware to actually async flip? */
952+
bool do_async_flip;
953+
954954
/* Pipe source size (ie. panel fitter input size)
955955
* All planes will be positioned inside this space,
956956
* and get clipped at the edges. */

0 commit comments

Comments
 (0)