> Subject: [PATCH v6 3/5] drm/atomic: Allocate atomic_state at the beginning of
> atomic_ioctl
> 
> Moving atomic_state allocation to the beginning of the atomci_ioctl to

This needs to be in imerative language so something like "Move ..."
Also Typo *atomic_ioctl

> accommodate drm_mode_atomic_err_code usage for returning error code on
> failures.
> 

Also maybe mention why drm_mode_atomic_err_code cannot be accommodated at 
previous place

> Signed-off-by: Arun R Murthy <[email protected]>
> ---
>  drivers/gpu/drm/drm_atomic_uapi.c | 21 +++++++++++----------
>  1 file changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
> b/drivers/gpu/drm/drm_atomic_uapi.c
> index
> 85dbdaa4a2e25878c953b9b41539c8566d55c6d9..3ef478e717bec917d1b8803c
> 72bbcc8d6409d745 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -1394,13 +1394,21 @@ int drm_mode_atomic_ioctl(struct drm_device
> *dev,
>       struct drm_modeset_acquire_ctx ctx;
>       struct drm_out_fence_state *fence_state;
>       int ret = 0;
> -     unsigned int i, j, num_fences;
> +     unsigned int i, j, num_fences = 0;
>       bool async_flip = false;
> 
>       /* disallow for drivers not supporting atomic: */
>       if (!drm_core_check_feature(dev, DRIVER_ATOMIC))
>               return -EOPNOTSUPP;
> 
> +     state = drm_atomic_state_alloc(dev);
> +     if (!state)
> +             return -ENOMEM;
> +
> +     drm_modeset_acquire_init(&ctx,
> DRM_MODESET_ACQUIRE_INTERRUPTIBLE);
> +     state->acquire_ctx = &ctx;
> +     state->allow_modeset = !!(arg->flags &
> DRM_MODE_ATOMIC_ALLOW_MODESET);
> +
>       /* disallow for userspace that has not enabled atomic cap (even
>        * though this may be a bit overkill, since legacy userspace
>        * wouldn't know how to call this ioctl) @@ -1439,14 +1447,6 @@ int
> drm_mode_atomic_ioctl(struct drm_device *dev,
>               return -EINVAL;
>       }
> 
> -     state = drm_atomic_state_alloc(dev);
> -     if (!state)
> -             return -ENOMEM;
> -
> -     drm_modeset_acquire_init(&ctx,
> DRM_MODESET_ACQUIRE_INTERRUPTIBLE);
> -     state->acquire_ctx = &ctx;
> -     state->allow_modeset = !!(arg->flags &
> DRM_MODE_ATOMIC_ALLOW_MODESET);
> -
>  retry:
>       copied_objs = 0;
>       copied_props = 0;
> @@ -1543,7 +1543,8 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
>       }
> 
>  out:
> -     complete_signaling(dev, state, fence_state, num_fences, !ret);
> +     if (num_fences)
> +             complete_signaling(dev, state, fence_state, num_fences, !ret);

Why the need to check num_fences before we complete signalling
Also this seems like a separate change maybe should be in its own patch

Regards,
Suraj Kandpal

> 
>       if (ret == -EDEADLK) {
>               drm_atomic_state_clear(state);
> 
> --
> 2.25.1

Reply via email to