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

[Imre Deak <imre.deak@intel.com>]

On Mon, Nov 18, 2024 at 01:31:34PM +0530, Sharma, Swati2 wrote:
> 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.

Yes, it looks like the same issue.

> Anyways, patch LGTM
> 
> Reviewed-by: Swati Sharma <swati2.sharma@intel.com>

Thanks for thet review, patchset is pushed now adding the above closes
tag. The CI failures are unrelated, since all of those happen in a
different test than igt/kms_flip_tiling (in which these changes have no
effect).

> 
> 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. */