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

["Murthy, Arun R" <arun.r.murthy@intel.com>]

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.murthy@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