Hi,
On 4 August 2015 at 12:34, Maarten Lankhorst maarten.lankhorst@linux.intel.com wrote:
Commit ec9f932ed41622d120de52a5b525e4d77b9ef17e "drm/atomic: Cleanup on error properly in the atomic ioctl." cleaned up some error paths, but didn't fix the TEST_ONLY path. In the check only case plane->fb shouldn't be updated, and the vblank events should be cleared as on failure.
Bikeshedding a bit ...
An early test precludes TEST_ONLY | PAGE_FLIP_EVENT, so you don't need to mention this in the commit message; in this case, the main change is about plane->{,old_}fb.
@@ -1532,7 +1533,7 @@ retry: ret = drm_atomic_check_only(state); /* _check_only() does not free state, unlike _commit() */ if (!ret)
drm_atomic_state_free(state);
goto free; } else if (arg->flags & DRM_MODE_ATOMIC_NONBLOCK) { ret = drm_atomic_async_commit(state); } else {@@ -1566,6 +1567,7 @@ out: }
if (ret) {+free:
This is a bit nasty. Can we please move the label above the conditional and change the condition to (ret || flags & TEST_ONLY)? Doing that, you could also move the label above the (ret == -EDEADLK) check, which would cover ->atomic_check needing to grab other states (global resources?) and failing.
Cheers, Daniel