[igt-dev] [PATCH] tests/kms_flip: Do not check for timestamp or sequences on Mediatek
Petri Latvala
petri.latvala at intel.com
Tue Dec 7 16:03:15 UTC 2021
On Tue, Dec 07, 2021 at 10:46:44AM -0500, Mark Yacoub wrote:
> On Tue, Dec 7, 2021 at 10:42 AM Petri Latvala <petri.latvala at intel.com> wrote:
> >
> > On Fri, Dec 03, 2021 at 02:58:24PM -0500, Mark Yacoub wrote:
> > > From: Mark Yacoub <markyacoub at google.com>
> > >
> > > [Why]
> > > Mediatek devices have a HW issue with sending their vblank IRQ at the same time interval
> > > everytime. The drift can be below or above the expected frame time, causing the
> > > timestamp to drift with a relatively larger standard deviation over a large sample.
> > >
> > > [How]
> > > Filter out the flags TEST_CHECK_TS and TEST_VBLANK_EXPIRED_SEQ from the
> > > tests flags, and restrict sequence and ts checks.
> > >
> > > Tested on Jacuzzi (MT8183)
> > >
> > > Signed-off-by: Mark Yacoub <markyacoub at chromium.org>
> > > ---
> > > lib/drmtest.c | 5 +++++
> > > lib/drmtest.h | 1 +
> > > tests/kms_flip.c | 44 ++++++++++++++++++++++++++++++++------------
> > > 3 files changed, 38 insertions(+), 12 deletions(-)
> > >
> > > diff --git a/lib/drmtest.c b/lib/drmtest.c
> > > index 29cb3f4c..09a9a229 100644
> > > --- a/lib/drmtest.c
> > > +++ b/lib/drmtest.c
> > > @@ -114,6 +114,11 @@ bool is_i915_device(int fd)
> > > return __is_device(fd, "i915");
> > > }
> > >
> > > +bool is_mtk_device(int fd)
> > > +{
> > > + return __is_device(fd, "mediatek");
> > > +}
> > > +
> > > bool is_msm_device(int fd)
> > > {
> > > return __is_device(fd, "msm");
> > > diff --git a/lib/drmtest.h b/lib/drmtest.h
> > > index a6eb60c3..b5debd44 100644
> > > --- a/lib/drmtest.h
> > > +++ b/lib/drmtest.h
> > > @@ -103,6 +103,7 @@ void igt_require_vc4(int fd);
> > >
> > > bool is_amdgpu_device(int fd);
> > > bool is_i915_device(int fd);
> > > +bool is_mtk_device(int fd);
> > > bool is_msm_device(int fd);
> > > bool is_nouveau_device(int fd);
> > > bool is_vc4_device(int fd);
> > > diff --git a/tests/kms_flip.c b/tests/kms_flip.c
> > > index ccb085da..56addab8 100755
> > > --- a/tests/kms_flip.c
> > > +++ b/tests/kms_flip.c
> > > @@ -129,6 +129,15 @@ struct event_state {
> > > int seq_step;
> > > };
> > >
> > > +static bool should_skip_ts_checks() {
> > > + /* Mediatek devices have a HW issue with sending their vblank IRQ at the same time interval
> > > + * everytime. The drift can be below or above the expected frame time, causing the
> > > + * timestamp to drift with a relatively larger standard deviation over a large sample.
> > > + * As it's a known issue, skip any Timestamp or Sequence checks for MTK drivers.
> > > + */
> > > + return is_mtk_device(drm_fd);
> > > +}
> > > +
> > > static bool vblank_dependence(int flags)
> > > {
> > > int vblank_flags = TEST_VBLANK | TEST_VBLANK_BLOCK |
> > > @@ -761,16 +770,19 @@ static bool run_test_step(struct test_output *o, unsigned int *events)
> > > start = gettime_us();
> > > igt_assert(__wait_for_vblank(TEST_VBLANK_BLOCK, o->pipe, 2, 0, &reply) == 0);
> > > end = gettime_us();
> > > - /*
> > > - * we waited for two vblanks, so verify that
> > > - * we were blocked for ~1-2 frames. And due
> > > - * to scheduling latencies we give it an extra
> > > - * half a frame or so.
> > > - */
> > > - igt_assert_f(end - start > 0.9 * actual_frame_time(o) &&
> > > - end - start < 2.6 * actual_frame_time(o),
> > > - "wait for two vblanks took %lu usec (frame time %f usec)\n",
> > > - end - start, mode_frame_time(o));
> > > +
> > > + if (!should_skip_ts_checks()) {
> > > + /*
> > > + * we waited for two vblanks, so verify that
> > > + * we were blocked for ~1-2 frames. And due
> > > + * to scheduling latencies we give it an extra
> > > + * half a frame or so.
> > > + */
> > > + igt_assert_f(end - start > 0.9 * actual_frame_time(o) &&
> > > + end - start < 2.6 * actual_frame_time(o),
> > > + "wait for two vblanks took %lu usec (frame time %f usec)\n",
> > > + end - start, mode_frame_time(o));
> > > + }
> > > join_vblank_wait_thread();
> > > }
> > >
> > > @@ -1228,8 +1240,10 @@ static bool calibrate_ts(struct test_output *o, int crtc_idx)
> > >
> > > igt_info("Expected frametime: %.0fus; measured %.1fus +- %.3fus accuracy %.2f%%\n",
> > > expected, mean, stddev, 100 * 3 * stddev / mean);
> > > - /* 99.7% samples within 0.5% of the mean */
> > > - igt_assert(3 * stddev / mean < 0.005);
> > > + if (!should_skip_ts_checks())
> > > + /* 99.7% samples within 0.5% of the mean */
> > > + igt_assert(3 * stddev / mean < 0.005);
> >
> > Put that igt_info above into the if block as well, it's useless if
> > it's not checked, right?
> >
> >
> > > +
> > > /* 84% samples within 0.5% of the expected value.
> > > * See comments in check_timings() in kms_setmode.c
> > > */
> > > @@ -1723,6 +1737,12 @@ igt_main_args("e", NULL, help_str, opt_handler, NULL)
> > > if (is_i915_device(drm_fd)) {
> > > bops = buf_ops_create(drm_fd);
> > > }
> > > +
> > > + if (should_skip_ts_checks()) {
> > > + igt_info("Skipping timestamp checks\n");
> > > + for (i = 0; i < sizeof(tests) / sizeof(tests[0]); i++)
> > > + tests[i].flags &= ~(TEST_CHECK_TS | TEST_VBLANK_EXPIRED_SEQ);
> > > + }
> > > }
> > >
> >
> > Tentatively, this is
> > Reviewed-by: Petri Latvala <petri.latvala at intel.com>
> >
> > With those asserts removed, is there still enough left for mtk devices
> > that running the tests have some merit?
> yeah there is a fair number of igt_asserts that the tests hit, it runs
> for quite sometime.
Excellent! The r-b stands then.
--
Petri Latvala
More information about the igt-dev
mailing list