@@ -13882,6 +13882,25 @@ static void intel_atomic_commit_work(struct work_struct *work)
intel_atomic_commit_tail(state);
}
+static void intel_atomic_track_fbs(struct drm_atomic_state *state)
+{
+ struct drm_plane_state *old_plane_state;
+ struct drm_plane *plane;
+ struct drm_i915_gem_object *obj, *old_obj;
+ struct intel_plane *intel_plane;
+ int i;
+
+ mutex_lock(&state->dev->struct_mutex);
+ for_each_plane_in_state(state, plane, old_plane_state, i) {
+ obj = intel_fb_obj(plane->state->fb);
+ old_obj = intel_fb_obj(old_plane_state->fb);
+ intel_plane = to_intel_plane(plane);
+
+ i915_gem_track_fb(old_obj, obj, intel_plane->frontbuffer_bit);
+ }
+ mutex_unlock(&state->dev->struct_mutex);
+}
+
/**
* intel_atomic_commit - commit validated state object
* @dev: DRM device
@@ -13922,6 +13941,7 @@ static int intel_atomic_commit(struct drm_device *dev,
dev_priv->wm.distrust_bios_wm = false;
dev_priv->wm.skl_results = intel_state->wm_results;
intel_shared_dpll_commit(state);
+ intel_atomic_track_fbs(state);
if (nonblock)
queue_work(system_unbound_wq, &state->commit_work);
@@ -14001,7 +14021,6 @@ intel_prepare_plane_fb(struct drm_plane *plane,
{
struct drm_device *dev = plane->dev;
struct drm_framebuffer *fb = new_state->fb;
- struct intel_plane *intel_plane = to_intel_plane(plane);
struct drm_i915_gem_object *obj = intel_fb_obj(fb);
struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->state->fb);
int ret = 0;
@@ -14058,16 +14077,12 @@ intel_prepare_plane_fb(struct drm_plane *plane,
ret = intel_pin_and_fence_fb_obj(fb, new_state->rotation);
}
- if (ret == 0) {
- if (obj) {
- struct intel_plane_state *plane_state =
- to_intel_plane_state(new_state);
-
- i915_gem_request_assign(&plane_state->wait_req,
- obj->last_write_req);
- }
+ if (ret == 0 && obj) {
+ struct intel_plane_state *plane_state =
+ to_intel_plane_state(new_state);
- i915_gem_track_fb(old_obj, obj, intel_plane->frontbuffer_bit);
+ i915_gem_request_assign(&plane_state->wait_req,
+ obj->last_write_req);
}
return ret;
@@ -14087,7 +14102,6 @@ intel_cleanup_plane_fb(struct drm_plane *plane,
const struct drm_plane_state *old_state)
{
struct drm_device *dev = plane->dev;
- struct intel_plane *intel_plane = to_intel_plane(plane);
struct intel_plane_state *old_intel_state;
struct drm_i915_gem_object *old_obj = intel_fb_obj(old_state->fb);
struct drm_i915_gem_object *obj = intel_fb_obj(plane->state->fb);
@@ -14101,11 +14115,6 @@ intel_cleanup_plane_fb(struct drm_plane *plane,
!INTEL_INFO(dev)->cursor_needs_physical))
intel_unpin_fb_obj(old_state->fb, old_state->rotation);
- /* prepare_fb aborted? */
- if ((old_obj && (old_obj->frontbuffer_bits & intel_plane->frontbuffer_bit)) ||
- (obj && !(obj->frontbuffer_bits & intel_plane->frontbuffer_bit)))
- i915_gem_track_fb(old_obj, obj, intel_plane->frontbuffer_bit);
-
i915_gem_request_assign(&old_intel_state->wait_req, NULL);
}
Currently it's part of prepare_fb, still in the first phase of atomic_commit which might fail. Which means that we need to have some heuristics in cleanup_fb to figure out whether things failed, or whether we just clean up the old fbs. That's fragile, and worse, once we start pipelining commits gets confused: While the last commit is still getting cleanup up we already hammer in the new one, and fb_bits aren't refcounted, resulting in lost bits and WARN_ON galore. We could instead try to make cleanup_fb more clever, but a simpler fix is to postpone the fb_bits tracking past the point of no return, where we commit all the software state. That also makes conceptually more sense, since fb_bits must be updated synchronously from the ioctl (they track usage from userspace pov, not from the hw pov), right before we're fully committed to the updated. This fixes WARNING splats from track_fb with page_flip implemented through atomic_commit. Testcase: igt/kms_flip/flip-vs-rmfb Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> --- drivers/gpu/drm/i915/intel_display.c | 41 ++++++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 16 deletions(-)