Op 25-07-17 om 11:27 schreef Daniel Vetter:
On Tue, Jul 25, 2017 at 11:11 AM, Maarten Lankhorst maarten.lankhorst@linux.intel.com wrote:
Op 25-07-17 om 10:23 schreef Daniel Vetter:
On Wed, Jul 19, 2017 at 04:39:15PM +0200, Maarten Lankhorst wrote:
/*
* Don't do an async update if there is an outstanding commit modifying* the plane. This prevents our async update's changes from getting* overridden by a previous synchronous update's state.
* FIXME: We should prevent an async update if there is an outstanding* commit modifying the plane. This prevents our async update's* changes from getting overwritten by a previous synchronous update's* state. */
- for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
if (plane->crtc != crtc)continue;spin_lock(&crtc->commit_lock);commit = list_first_entry_or_null(&crtc->commit_list,struct drm_crtc_commit,commit_entry);if (!commit) {spin_unlock(&crtc->commit_lock);continue;}spin_unlock(&crtc->commit_lock);if (!crtc->state->state)continue;for_each_plane_in_state(crtc->state->state, __plane,__plane_state, j) {I'm pretty sure this oopses, because crtc->state->state is NULL after commit. I think Gustavo needs to review this first (and write a nasty igt testcase to catch it) before we remove this. I think the correct check is to simply bail out if our current crtc has a pending commit (i.e. !list_empty(&crtc->commit_list) should be all we need to check.
It didn't oops. Right above it was a null check. It was never executed. :)
obj->state->state is always NULL. Excluding a brief moment during swap_state where this may oops, this code willl never do a thing.
Oh right. It's still completely buggy, and I'd like to fix that first, testcase included. Can you pls poke Gustavo a bit (or maybe he's on vacation)? -Daniel
The only thing we have atm excercising it is kms_cursor_legacy, but that doesn't check if flips can be overwritten. Perhaps we should make IGT tests a requirement for features in the future?