Commit ec9f932e authored by Maarten Lankhorst's avatar Maarten Lankhorst Committed by Daniel Vetter

drm/atomic: Cleanup on error properly in the atomic ioctl.

It's probably allowed to leave old_fb set to garbage when unlocking,
but to prevent undefined behavior unset it just in case.

Also crtc_state->event could be NULL on memory allocation failure,
in which case event_space is increased for no reason.

Note: Contains some general simplification of the cleanup code too.
Signed-off-by: default avatarMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
[danvet: Add note about the other changes in here. And fix long line
while at it.]
Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
parent 3cb43cc0
...@@ -1463,18 +1463,18 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, ...@@ -1463,18 +1463,18 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
if (get_user(obj_id, objs_ptr + copied_objs)) { if (get_user(obj_id, objs_ptr + copied_objs)) {
ret = -EFAULT; ret = -EFAULT;
goto fail; goto out;
} }
obj = drm_mode_object_find(dev, obj_id, DRM_MODE_OBJECT_ANY); obj = drm_mode_object_find(dev, obj_id, DRM_MODE_OBJECT_ANY);
if (!obj || !obj->properties) { if (!obj || !obj->properties) {
ret = -ENOENT; ret = -ENOENT;
goto fail; goto out;
} }
if (get_user(count_props, count_props_ptr + copied_objs)) { if (get_user(count_props, count_props_ptr + copied_objs)) {
ret = -EFAULT; ret = -EFAULT;
goto fail; goto out;
} }
copied_objs++; copied_objs++;
...@@ -1486,25 +1486,25 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, ...@@ -1486,25 +1486,25 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
if (get_user(prop_id, props_ptr + copied_props)) { if (get_user(prop_id, props_ptr + copied_props)) {
ret = -EFAULT; ret = -EFAULT;
goto fail; goto out;
} }
prop = drm_property_find(dev, prop_id); prop = drm_property_find(dev, prop_id);
if (!prop) { if (!prop) {
ret = -ENOENT; ret = -ENOENT;
goto fail; goto out;
} }
if (copy_from_user(&prop_value, if (copy_from_user(&prop_value,
prop_values_ptr + copied_props, prop_values_ptr + copied_props,
sizeof(prop_value))) { sizeof(prop_value))) {
ret = -EFAULT; ret = -EFAULT;
goto fail; goto out;
} }
ret = atomic_set_prop(state, obj, prop, prop_value); ret = atomic_set_prop(state, obj, prop, prop_value);
if (ret) if (ret)
goto fail; goto out;
copied_props++; copied_props++;
} }
...@@ -1523,7 +1523,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, ...@@ -1523,7 +1523,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
e = create_vblank_event(dev, file_priv, arg->user_data); e = create_vblank_event(dev, file_priv, arg->user_data);
if (!e) { if (!e) {
ret = -ENOMEM; ret = -ENOMEM;
goto fail; goto out;
} }
crtc_state->event = e; crtc_state->event = e;
...@@ -1533,13 +1533,15 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, ...@@ -1533,13 +1533,15 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
if (arg->flags & DRM_MODE_ATOMIC_TEST_ONLY) { if (arg->flags & DRM_MODE_ATOMIC_TEST_ONLY) {
ret = drm_atomic_check_only(state); ret = drm_atomic_check_only(state);
/* _check_only() does not free state, unlike _commit() */ /* _check_only() does not free state, unlike _commit() */
drm_atomic_state_free(state); if (!ret)
drm_atomic_state_free(state);
} else if (arg->flags & DRM_MODE_ATOMIC_NONBLOCK) { } else if (arg->flags & DRM_MODE_ATOMIC_NONBLOCK) {
ret = drm_atomic_async_commit(state); ret = drm_atomic_async_commit(state);
} else { } else {
ret = drm_atomic_commit(state); ret = drm_atomic_commit(state);
} }
out:
/* if succeeded, fixup legacy plane crtc/fb ptrs before dropping /* if succeeded, fixup legacy plane crtc/fb ptrs before dropping
* locks (ie. while it is still safe to deref plane->state). We * locks (ie. while it is still safe to deref plane->state). We
* need to do this here because the driver entry points cannot * need to do this here because the driver entry points cannot
...@@ -1552,41 +1554,35 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, ...@@ -1552,41 +1554,35 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
drm_framebuffer_reference(new_fb); drm_framebuffer_reference(new_fb);
plane->fb = new_fb; plane->fb = new_fb;
plane->crtc = plane->state->crtc; plane->crtc = plane->state->crtc;
} else {
plane->old_fb = NULL; if (plane->old_fb)
} drm_framebuffer_unreference(plane->old_fb);
if (plane->old_fb) {
drm_framebuffer_unreference(plane->old_fb);
plane->old_fb = NULL;
} }
plane->old_fb = NULL;
} }
drm_modeset_drop_locks(&ctx); if (ret == -EDEADLK) {
drm_modeset_acquire_fini(&ctx); drm_atomic_state_clear(state);
drm_modeset_backoff(&ctx);
return ret; goto retry;
}
fail: if (ret) {
if (ret == -EDEADLK) if (arg->flags & DRM_MODE_PAGE_FLIP_EVENT) {
goto backoff; for_each_crtc_in_state(state, crtc, crtc_state, i) {
if (!crtc_state->event)
continue;
if (arg->flags & DRM_MODE_PAGE_FLIP_EVENT) { destroy_vblank_event(dev, file_priv,
for_each_crtc_in_state(state, crtc, crtc_state, i) { crtc_state->event);
destroy_vblank_event(dev, file_priv, crtc_state->event); }
crtc_state->event = NULL;
} }
}
drm_atomic_state_free(state); drm_atomic_state_free(state);
}
drm_modeset_drop_locks(&ctx); drm_modeset_drop_locks(&ctx);
drm_modeset_acquire_fini(&ctx); drm_modeset_acquire_fini(&ctx);
return ret; return ret;
backoff:
drm_atomic_state_clear(state);
drm_modeset_backoff(&ctx);
goto retry;
} }
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment