drm/i915: Move fb_bits updating later in atomic_commit

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
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1465827229-1704-4-git-send-email-daniel.vetter@ffwll.ch

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index b26e62473bbf66243065d8e124f0e10f2d2b4050..73af6c3b68311803224b32f2b9084ebf63ad80e9 100644 (file)
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13885,6 +13885,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
@@ -13930,6 +13949,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);
@@ -14009,7 +14029,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;
@@ -14066,16 +14085,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;
@@ -14095,7 +14110,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);
@@ -14109,11 +14123,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);
 }