| Message ID | 55C0A330.6050404@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index d719ce0b10a0..03672a782e84 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -1507,7 +1507,8 @@ retry: copied_props++; } - if (obj->type == DRM_MODE_OBJECT_PLANE && count_props) { + if (obj->type == DRM_MODE_OBJECT_PLANE && count_props && + !(arg->flags & DRM_MODE_ATOMIC_TEST_ONLY)) { plane = obj_to_plane(obj); plane_mask |= (1 << drm_plane_index(plane)); plane->old_fb = plane->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: if (arg->flags & DRM_MODE_PAGE_FLIP_EVENT) { for_each_crtc_in_state(state, crtc, crtc_state, i) { if (!crtc_state->event)
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. Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> ---