Re: [PATCH i-g-t 3/3] tests/i915/kms_flip_tiling: Fix pageflip timeout for low vrefresh modes

["Sharma, Swati2" <swati2.sharma@intel.com>]

Imre, Can you check if your patch is fixing following gitlab issue

https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/1491

This is MST config re-lnl-4 with 4K@30 mode

https://intel-gfx-ci.01.org/tree/intel-xe/xe-2244-5b1f614ccf1008e7aee531bf76bb639802e0df88/re-lnl-4/igt@kms_flip_tiling@flip-change-tiling.html

If so, add closes tag.

Anyways, patch LGTM

Reviewed-by: Swati Sharma <swati2.sharma@intel.com>

On 15-11-2024 10:41 pm, Imre Deak wrote:

> On Fri, Nov 15, 2024 at 05:14:22PM +0530, Sharma, Swati2 wrote:
>> Hi Imre,
>>
>> Do we have some gitlab issue associated with this fix?
> Not sure if there is, the problem needs a mode with a vrefresh rate <
> 60Hz (to be added to CI for an MST config at least).
>
>> On 22-10-2024 09:23 pm, Imre Deak wrote:
>>> The current fixed 50 ms timeout for pageflip completion is not long
>>> enough for low vrefresh modes. In general the timeout should account for
>>> one frame for the flip itself + one frame for vblank waiting due to FBC
>>> invalidation + a fixed amount for scheduling overhead delivering the
>>> completion event to userspace. For the usual 60Hz modes the 50 ms
>>> timeout worked ok based on the above (2 * 17 ms + 16 ms), but didn't
>>> work for 30 Hz modes (the minimum timeout for those being ~67 ms).
>>>
>>> Based on the above use a timeout length of 2 * mode specific frame
>>> time + 20 ms fixed scheduling delay.
>>>
>>> Testcase: igt@kms_flip_tiling
>>> Signed-off-by: Imre Deak <imre.deak@intel.com>
>>> ---
>>>    tests/intel/kms_flip_tiling.c | 14 +++++++++++++-
>>>    1 file changed, 13 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/tests/intel/kms_flip_tiling.c b/tests/intel/kms_flip_tiling.c
>>> index 3bf02564e..e937c2171 100644
>>> --- a/tests/intel/kms_flip_tiling.c
>>> +++ b/tests/intel/kms_flip_tiling.c
>>> @@ -86,6 +86,18 @@ static int try_commit(igt_display_t *display)
>>>    				       COMMIT_ATOMIC : COMMIT_LEGACY);
>>>    }
>>> +static uint64_t pageflip_timeout_us(drmModeModeInfo *mode)
>>> +{
>>> +	uint64_t timeout_ns;
>>> +
>>> +	/* 1 frame for flip + 1 frame for vblank wait due to FBC. */
>>> +	timeout_ns = igt_kms_frame_time_from_vrefresh(mode->vrefresh) * 2;
>>> +	/* 20 msec scheduling overhead. */
>>> +	timeout_ns += 20000000;
>>> +
>>> +	return DIV_ROUND_UP(timeout_ns, 1000);
>>> +}
>>> +
>>>    static void
>>>    test_flip_tiling(data_t *data, enum pipe pipe, igt_output_t *output, uint64_t modifier[2])
>>>    {
>>> @@ -137,7 +149,7 @@ test_flip_tiling(data_t *data, enum pipe pipe, igt_output_t *output, uint64_t mo
>>>    	igt_require(ret == 0);
>>>    	data->flipevent_in_queue = true;
>>> -	kmstest_wait_for_pageflip(data->drm_fd);
>>> +	kmstest_wait_for_pageflip_timeout(data->drm_fd, pageflip_timeout_us(mode));
>>>    	data->flipevent_in_queue = false;
>>>    	/* Get a crc and compare with the reference. */