Re: [PATCH v6 3/5] drm/atomic: Allocate atomic_state at the beginning of atomic_ioctl

[Murthy, Arun R]

On 29-10-2025 13:16, Kandpal, Suraj wrote:
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
Corrected!
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
Done!
Signed-off-by: Arun R Murthy <arun.r.mur...@intel.com>
---
  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

Even before creating signals there are few error checks where we return 
back with failure code, hence checking for signals created before 
signalling.

Thanks and Regards,
Arun R Murthy
-------------------

Regards,
Suraj Kandpal

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

--
2.25.1