On Wed, Jun 24, 2015 at 08:59:25AM +0200, Maarten Lankhorst wrote:
> 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.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankho...@linux.intel.com>
Grumpy reviewer comment: You've failed to mention that you rework a few
things just because, makes it harder to spot the actual changes. Anyway,
applied to drm-misc.
-Daniel
> ---
> drivers/gpu/drm/drm_atomic.c | 63
> ++++++++++++++++++++------------------------
> 1 file changed, 29 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index bd7f723c708e..a89f73f1dca1 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,34 @@ 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;
> }
> --
> 2.1.0
>
> _______________________________________________
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
