Re: [PATCH i-g-t] tests/kms_explicit_fence: Rescue pending IN_FENCE waits on test failures

[Karthik B S <karthik.b.s@intel.com>]

Hi Krzysztof,

On 5/5/2026 12:32 PM, Krzysztof Karas wrote:
> Hi Karthik,
>
> [...]
>
>> +static struct {
>> +	int timelines[NUM_PLANES];
>> +	bool needs_signal[NUM_PLANES];
>> +	bool active;
> I wonder if this flag is needed. See below.
>
>> +} pending_fence_state = {
>> +	.timelines = { -1, -1, -1 },
>> +};
>> +
>> +static void signal_pending_fences(int sig)
>> +{
>> +	if (!pending_fence_state.active)
>> +		return;
> The information about not needing to signal could be derived
> from the fact that no timeline has been assigned.

Thank you for the review.

Agreed, will fix this in v2.

>
>> +
>> +	for (int i = 0; i < NUM_PLANES; i++) {
>> +		if (pending_fence_state.timelines[i] < 0)
>> +			continue;
>> +
>> +		if (pending_fence_state.needs_signal[i]) {
>> +			/*
>> +			 * Best-effort: advance the timeline so the kernel
>> +			 * stops waiting on this IN_FENCE.
>> +			 */
>> +			sw_sync_timeline_inc(pending_fence_state.timelines[i], 1);
>> +			pending_fence_state.needs_signal[i] = false;
>> +		}
>> +	}
>> +
>> +	pending_fence_state.active = false;
>> +}
>> +
>>   static void setup_output(data_t *data)
>>   {
>>   	igt_display_t *display = &data->display;
>> @@ -242,8 +278,14 @@ static void multiplane_atomic_fence_wait(data_t *data)
>>   
>>   		/* Attach IN_FENCE_FD to plane */
>>   		igt_plane_set_fence_fd(planes[i], fences[i]);
>> +
>> +		pending_fence_state.timelines[i] = timelines[i];
>> +		pending_fence_state.needs_signal[i] = !should_signal[i];
>>   	}
>>   
>> +	/* Arm the exit handler before the NONBLOCK commit goes out. */
>> +	pending_fence_state.active = true;
> Can "signal_pending_fences()" be called mid initialization of
> "struct pending_fence_state"? If not, then this is truly
> redundant.
Agreed, this is redundant. Will remove this variable.
>
>> +
>>   	/* Swap overlay colors to detect scanout changes via CRC */
>>   	igt_plane_set_fb(data->overlay1, &data->overlay2_fb);
>>   	igt_plane_set_position(data->overlay1, OVERLAY1_POS_X, OVERLAY1_POS_Y);
>> @@ -299,6 +341,7 @@ static void multiplane_atomic_fence_wait(data_t *data)
>>   	igt_assert_crc_equal(&crc_before, &crc_after);
>>   
>>   	/* Now signal the blocking fence (overlay2) */
>> +	pending_fence_state.needs_signal[PLANE_OVERLAY2_IDX] = false;
>>   	sw_sync_timeline_inc(timelines[PLANE_OVERLAY2_IDX], 1);
>>   
>>   	/* Wait for overlay2 fence to be signaled */
>> @@ -311,6 +354,12 @@ static void multiplane_atomic_fence_wait(data_t *data)
>>   	igt_assert_eq(ret, 0);
>>   	igt_assert_eq(sync_fence_status(out_fence), 1);
>>   
>> +	/*
>> +	 * The pending atomic commit has now completed, so the exit handler
>> +	 * no longer needs to rescue it.
>> +	 */
>> +	pending_fence_state.active = false;
>> +
>>   	/* Verify display has now updated (CRC should differ from baseline) */
>>   	igt_pipe_crc_get_current(data->drm_fd, data->pipe_crc, &crc_final);
>>   
>> @@ -353,6 +402,13 @@ int igt_main()
>>   
>>   		data.pipe_crc = igt_pipe_crc_new(data.drm_fd, data.crtc->crtc_index,
>>   						 IGT_PIPE_CRC_SOURCE_AUTO);
>> +
>> +		/*
>> +		 * Make sure that on any failure mid-test, any unsignaled
>> +		 * IN_FENCE that the kernel is still waiting on gets
>> +		 * signaled so cleanup can complete instead of hanging.
>> +		 */
>> +		igt_install_exit_handler(signal_pending_fences);
>>   	}
>>   
>>   	igt_describe("Test atomic commit with 3 planes (1 primary, 2 overlay) "
>> @@ -363,6 +419,8 @@ int igt_main()
>>   		multiplane_atomic_fence_wait(&data);
>>   
>>   	igt_fixture() {
>> +		signal_pending_fences(0);
>> +
> You could skip this extra line.

Will update this in v2.

Thanks and Regards,
Karthik.B.S
>
>>   		reset_display_state(&data);
>>   		igt_pipe_crc_free(data.pipe_crc);
>>   		cleanup_crtc(&data);
>> -- 
>> 2.43.0
>>