Skip to content

Commit

Permalink
drm/i915: Move fb_bits updating later in atomic_commit
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Daniel Vetter committed Jun 16, 2016
1 parent 94f0502 commit 6c9c1b3
Showing 1 changed file with 25 additions and 16 deletions.
41 changes: 25 additions & 16 deletions drivers/gpu/drm/i915/intel_display.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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);
Expand All @@ -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);
}

Expand Down

0 comments on commit 6c9c1b3

Please sign in to comment.