From a9cc54eeba635d2feb2ed4bf43d4ea4e08e7e0a9 Mon Sep 17 00:00:00 2001 From: Maarten Lankhorst Date: Wed, 24 Jun 2015 08:59:24 +0200 Subject: drm/atomic: Update old_fb after setting a property. This change updates the old_fb pointer only after acquiring the plane lock, if there are no properties the fb cannot have been changed either, so this works out correctly. Found in a discussion with Rob Clark. Cc: Rob Clark Signed-off-by: Maarten Lankhorst Reviewed-by: Rob Clark Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_atomic.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'drivers/gpu/drm/drm_atomic.c') diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index f6f2fb58eb37..bd7f723c708e 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -1472,12 +1472,6 @@ retry: goto fail; } - if (obj->type == DRM_MODE_OBJECT_PLANE) { - plane = obj_to_plane(obj); - plane_mask |= (1 << drm_plane_index(plane)); - plane->old_fb = plane->fb; - } - if (get_user(count_props, count_props_ptr + copied_objs)) { ret = -EFAULT; goto fail; @@ -1514,6 +1508,12 @@ retry: copied_props++; } + + if (obj->type == DRM_MODE_OBJECT_PLANE && count_props) { + plane = obj_to_plane(obj); + plane_mask |= (1 << drm_plane_index(plane)); + plane->old_fb = plane->fb; + } } if (arg->flags & DRM_MODE_PAGE_FLIP_EVENT) { -- cgit v1.2.3-59-g8ed1b From ec9f932ed41622d120de52a5b525e4d77b9ef17e Mon Sep 17 00:00:00 2001 From: Maarten Lankhorst Date: Wed, 24 Jun 2015 08:59:25 +0200 Subject: 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: Maarten Lankhorst [danvet: Add note about the other changes in here. And fix long line while at it.] Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_atomic.c | 64 +++++++++++++++++++++----------------------- 1 file changed, 30 insertions(+), 34 deletions(-) (limited to 'drivers/gpu/drm/drm_atomic.c') diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index bd7f723c708e..acebd1617264 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -1463,18 +1463,18 @@ retry: if (get_user(obj_id, objs_ptr + copied_objs)) { ret = -EFAULT; - goto fail; + goto out; } obj = drm_mode_object_find(dev, obj_id, DRM_MODE_OBJECT_ANY); if (!obj || !obj->properties) { ret = -ENOENT; - goto fail; + goto out; } if (get_user(count_props, count_props_ptr + copied_objs)) { ret = -EFAULT; - goto fail; + goto out; } copied_objs++; @@ -1486,25 +1486,25 @@ retry: if (get_user(prop_id, props_ptr + copied_props)) { ret = -EFAULT; - goto fail; + goto out; } prop = drm_property_find(dev, prop_id); if (!prop) { ret = -ENOENT; - goto fail; + goto out; } if (copy_from_user(&prop_value, prop_values_ptr + copied_props, sizeof(prop_value))) { ret = -EFAULT; - goto fail; + goto out; } ret = atomic_set_prop(state, obj, prop, prop_value); if (ret) - goto fail; + goto out; copied_props++; } @@ -1523,7 +1523,7 @@ retry: e = create_vblank_event(dev, file_priv, arg->user_data); if (!e) { ret = -ENOMEM; - goto fail; + goto out; } crtc_state->event = e; @@ -1533,13 +1533,15 @@ retry: if (arg->flags & DRM_MODE_ATOMIC_TEST_ONLY) { ret = drm_atomic_check_only(state); /* _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) { ret = drm_atomic_async_commit(state); } else { ret = drm_atomic_commit(state); } +out: /* if succeeded, fixup legacy plane crtc/fb ptrs before dropping * locks (ie. while it is still safe to deref plane->state). We * need to do this here because the driver entry points cannot @@ -1552,41 +1554,35 @@ retry: drm_framebuffer_reference(new_fb); plane->fb = new_fb; plane->crtc = plane->state->crtc; - } else { - plane->old_fb = NULL; - } - if (plane->old_fb) { - drm_framebuffer_unreference(plane->old_fb); - plane->old_fb = NULL; + + if (plane->old_fb) + drm_framebuffer_unreference(plane->old_fb); } + plane->old_fb = NULL; } - drm_modeset_drop_locks(&ctx); - drm_modeset_acquire_fini(&ctx); - - return ret; + if (ret == -EDEADLK) { + drm_atomic_state_clear(state); + drm_modeset_backoff(&ctx); + goto retry; + } -fail: - if (ret == -EDEADLK) - goto backoff; + if (ret) { + if (arg->flags & DRM_MODE_PAGE_FLIP_EVENT) { + for_each_crtc_in_state(state, crtc, crtc_state, i) { + if (!crtc_state->event) + continue; - if (arg->flags & DRM_MODE_PAGE_FLIP_EVENT) { - for_each_crtc_in_state(state, crtc, crtc_state, i) { - destroy_vblank_event(dev, file_priv, crtc_state->event); - crtc_state->event = NULL; + destroy_vblank_event(dev, file_priv, + crtc_state->event); + } } - } - drm_atomic_state_free(state); + drm_atomic_state_free(state); + } drm_modeset_drop_locks(&ctx); drm_modeset_acquire_fini(&ctx); return ret; - -backoff: - drm_atomic_state_clear(state); - drm_modeset_backoff(&ctx); - - goto retry; } -- cgit v1.2.3-59-g8ed1b