[igt-dev] [PATCH i-g-t] tests/kms_vrr: Update condition checks for flipline test
Modem, Bhanuprakash
bhanuprakash.modem at intel.com
Fri Jan 22 11:52:34 UTC 2021
> -----Original Message-----
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Sent: Wednesday, January 20, 2021 4:24 PM
> To: Modem, Bhanuprakash <bhanuprakash.modem at intel.com>
> Cc: igt-dev at lists.freedesktop.org; Navare, Manasi D
> <manasi.d.navare at intel.com>
> Subject: Re: [igt-dev] [PATCH i-g-t] tests/kms_vrr: Update condition
> checks for flipline test
>
> On Wed, Jan 20, 2021 at 05:36:59AM +0000, Modem, Bhanuprakash wrote:
> > > -----Original Message-----
> > > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > > Sent: Wednesday, January 20, 2021 12:46 AM
> > > To: Modem, Bhanuprakash <bhanuprakash.modem at intel.com>
> > > Cc: igt-dev at lists.freedesktop.org
> > > Subject: Re: [igt-dev] [PATCH i-g-t] tests/kms_vrr: Update condition
> > > checks for flipline test
> > >
> > > On Tue, Jan 19, 2021 at 07:47:12PM +0530, Bhanuprakash Modem wrote:
> > > > This patch includes below updates
> > > > * For Flipline test: if refresh_rate <= Vrr_min then
> > > > - Expected returned refresh rate would be vrr_max
> > > > - At least 35% of the flips should be in threshold
> > > > * Update "igt_display_commit_atomic" with "igt_display_commit2"
> > > > * Add few debug prints
> > >
> > > Some ideas I had for a vrr test:
> > > - test flipping at a few different rates
> > > - make sure the interval between flip timestamps
> > > matches the expected rate
> > Actually, we are doing the same in our flipline[*] test.
> >
> > Example: if we have a monitor with vrr range of 40 - 60Hz
> > We'll try to flip with 35, 50 and 65 Hz, and the difference between the
> > two flip timestamps should be within the required threshold from the
> > expected rate. A ~50us threshold is an arbitrary.
> >
> > [*] https://patchwork.freedesktop.org/patch/392037
> >
> > > - otherwise confirm the timestamps are sensible. Not quite sure what
> the
> > > best way would be. One idea was to just make sure the timestamp
> points
> > > to more or less the correct point in time. Another idea was to drive
> > > the whole loop baed on the timestamps + target refresh rate (ie.
> wait
> > > for event, schedule next flip for event timestamps + whatever time
> we
> > > have left to reach the exected refresh rate). This latter idea I
> think
> > > would catch bugs where the interval between timestamps is correct
> but
> > > the absolute times are not correct (eg. if the timestamps are
> > > miscorrected to point fr into the future based on the max vblank
> > > length, or too close into the future based on min vblank length).
> > > - the busy loop thing I don't think should be needed. Sure, there is
> > > some wakeup latency due to C-states and whatnot, but it should be
> > > possible to compensate to make sure we reach the target refresh rate
> > > w/o spinning.
> > This part is not clear to me.
> >
> > Ex. To generate the flips with refresh rate 50Hz (ie. 20000000ns), below
> > is the expected sequence:
> >
> > * Request a flip & wait for flip completion event
> > * Capture the event timestamp to compare
> > * Wait 20000000ns for next flip
>
> That would anyway need a bit of correction to account for event
> delivery latency + whatever overhead we have in submitting the
> flip.
>
> But in addition I was suggesting that we calculate how long to
> wait based on the flip event timestamp + whatever extra is needed
> on top to reach the target frame rate. Thus if the timestamp is
> garbage the test should fail since we'll end up flipping at the
> wrong rate.
Does this make sense?
- diff_ns = now_ns - start_ns;
+ diff_ns = last_event_ns - start_ns;
wait_ns = ((diff_ns + rate_ns - 1) / rate_ns) * rate_ns;
+ wait_ns += rate_ns - (last_event_ns - event_ns);
>
> > * Repeat
> >
> > Am I missing something here?
> >
> > >
> > > >
> > > > V2:
> > > > * Rebase
> > > >
> > > > Cc: Manasi Navare <manasi.d.navare at intel.com>
> > > > Cc: Nicholas Kazlauskas <nicholas.kazlauskas at amd.com>
> > > > Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem at intel.com>
> > > > ---
> > > > tests/kms_vrr.c | 51 ++++++++++++++++++++--------------------------
> ---
> > > > 1 file changed, 21 insertions(+), 30 deletions(-)
> > > >
> > > > diff --git a/tests/kms_vrr.c b/tests/kms_vrr.c
> > > > index beb06854f..e6edd131f 100644
> > > > --- a/tests/kms_vrr.c
> > > > +++ b/tests/kms_vrr.c
> > > > @@ -170,7 +170,7 @@ static void set_vrr_on_pipe(data_t *data, enum
> pipe
> > > pipe, bool enabled)
> > > > {
> > > > igt_pipe_set_prop_value(&data->display, pipe,
> IGT_CRTC_VRR_ENABLED,
> > > > enabled);
> > > > - igt_display_commit_atomic(&data->display, 0, NULL);
> > > > + igt_display_commit2(&data->display, COMMIT_ATOMIC);
> > > > }
> > > >
> > > > /* Prepare the display for testing on the given pipe. */
> > > > @@ -208,8 +208,7 @@ static void prepare_test(data_t *data,
> igt_output_t
> > > *output, enum pipe pipe)
> > > > */
> > > > igt_pipe_set_prop_value(&data->display, pipe,
> IGT_CRTC_VRR_ENABLED,
> > > 0);
> > > >
> > > > - igt_display_commit_atomic(&data->display,
> > > > - DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
> > > > + igt_display_commit2(&data->display, COMMIT_ATOMIC);
> > > > }
> > > >
> > > > /* Waits for the vblank interval. Returns the vblank timestamp in
> ns.
> > > */
> > > > @@ -291,14 +290,10 @@ flip_and_measure(data_t *data, igt_output_t
> > > *output, enum pipe pipe,
> > > > * difference between 144Hz and 143Hz which should give
> this
> > > > * enough accuracy for most use cases.
> > > > */
> > > > - if (rate_ns <= vtest_ns.min && rate_ns >= vtest_ns.max)
> > > > + if ((rate_ns < vtest_ns.min) && (rate_ns >=
> vtest_ns.max))
> > > > diff_ns = rate_ns;
> > > > - else if (rate_ns > vtest_ns.min)
> > > > - diff_ns = vtest_ns.min;
> > > > - else if (rate_ns < vtest_ns.max)
> > > > - diff_ns = vtest_ns.max;
> > > > else
> > > > - diff_ns = rate_ns;
> > > > + diff_ns = vtest_ns.max;
> > > > diff_ns -= event_ns - last_event_ns;
> > > >
> > > > if (llabs(diff_ns) < 50000ll)
> > > > @@ -323,8 +318,8 @@ flip_and_measure(data_t *data, igt_output_t
> *output,
> > > enum pipe pipe,
> > > > while (get_time_ns() < target_ns);
> > > > }
> > > >
> > > > - igt_info("Completed %u flips, %u were in threshold for
> > > %"PRIu64"ns.\n",
> > > > - total_flip, total_pass, rate_ns);
> > > > + igt_info("Completed %u flips, %u were in threshold for (%llu
> Hz)
> > > %"PRIu64"ns.\n",
> > > > + total_flip, total_pass, (NSECS_PER_SEC/rate_ns),
> rate_ns);
> > > >
> > > > return total_flip ? ((total_pass * 100) / total_flip) : 0;
> > > > }
> > > > @@ -338,8 +333,8 @@ test_basic(data_t *data, enum pipe pipe,
> > > igt_output_t *output, uint32_t flags)
> > > > range_t range = get_vrr_range(data, output);
> > > > uint64_t rate = vtest_ns.mid;
> > > >
> > > > - igt_info("VRR Test execution on %s, PIPE_%s\n",
> > > > - output->name, kmstest_pipe_name(pipe));
> > > > + igt_info("VRR Test execution on %s, PIPE_%s with VRR range:
> (%u-%u)
> > > Hz\n",
> > > > + output->name, kmstest_pipe_name(pipe), range.min,
> range.max);
> > > >
> > > > prepare_test(data, output, pipe);
> > > >
> > > > @@ -370,46 +365,42 @@ test_basic(data_t *data, enum pipe pipe,
> > > igt_output_t *output, uint32_t flags)
> > > > * decision boundary.
> > > > *
> > > > * Example: if range is 40 - 60Hz and
> > > > - * if refresh_rate > 60Hz:
> > > > + * if refresh_rate > 60Hz or <= 40Hz:
> > > > * Flip should happen at the flipline boundary & returned
> > > refresh rate
> > > > * would be 60Hz.
> > > > * if refresh_rate is 50Hz:
> > > > * Flip will happen right away so returned refresh rate is
> > > 50Hz.
> > > > - * if refresh_rate < 40Hz:
> > > > - * Flip should happen at the vmax so the returned refresh
> rate
> > > > - * would be 40Hz.
> > > > */
> > > > if (flags & TEST_FLIPLINE) {
> > > > - rate = rate_from_refresh(range.min - 5);
> > > > + rate = rate_from_refresh(range.max + 5);
> > > > result = flip_and_measure(data, output, pipe, rate,
> > > TEST_DURATION_NS);
> > > > igt_assert_f(result > 75,
> > > > - "Refresh rate %"PRIu64"ns: Target VRR on
> threshold
> > > not reached, result was %u%%\n",
> > > > - rate, result);
> > > > + "Refresh rate (%u Hz) %"PRIu64"ns: Target VRR
> on
> > > threshold not reached, result was %u%%\n",
> > > > + (range.max + 5), rate, result);
> > > >
> > > > - rate = rate_from_refresh(range.max + 5);
> > > > + rate = rate_from_refresh(range.min - 5);
> > > > result = flip_and_measure(data, output, pipe, rate,
> > > TEST_DURATION_NS);
> > > > - igt_assert_f(result > 75,
> > > > - "Refresh rate %"PRIu64"ns: Target VRR on
> threshold
> > > not reached, result was %u%%\n",
> > > > - rate, result);
> > > > + igt_assert_f(result > 35,
> > > > + "Refresh rate (%u Hz) %"PRIu64"ns: Target VRR
> on
> > > threshold not reached, result was %u%%\n",
> > > > + (range.min - 5), rate, result);
> > > > }
> > > >
> > > > rate = vtest_ns.mid;
> > > > result = flip_and_measure(data, output, pipe, rate,
> > > TEST_DURATION_NS);
> > > > igt_assert_f(result > 75,
> > > > - "Refresh rate %"PRIu64"ns: Target VRR on threshold
> not
> > > reached, result was %u%%\n",
> > > > - rate, result);
> > > > + "Refresh rate (%u Hz) %"PRIu64"ns: Target VRR on
> threshold
> > > not reached, result was %u%%\n",
> > > > + ((range.max + range.min) / 2), rate, result);
> > > >
> > > > set_vrr_on_pipe(data, pipe, 0);
> > > > result = flip_and_measure(data, output, pipe, rate,
> > > TEST_DURATION_NS);
> > > > igt_assert_f(result < 10,
> > > > - "Refresh rate %"PRIu64"ns: Target VRR off threshold
> > > exceeded, result was %u%%\n",
> > > > - rate, result);
> > > > + "Refresh rate (%u Hz) %"PRIu64"ns: Target VRR off
> > > threshold exceeded, result was %u%%\n",
> > > > + ((range.max + range.min) / 2), rate, result);
> > > >
> > > > /* Clean-up */
> > > > igt_plane_set_fb(data->primary, NULL);
> > > > igt_output_set_pipe(output, PIPE_NONE);
> > > > - igt_display_commit_atomic(&data->display,
> > > > - DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
> > > > + igt_display_commit2(&data->display, COMMIT_ATOMIC);
> > > >
> > > > igt_remove_fb(data->drm_fd, &data->fb1);
> > > > igt_remove_fb(data->drm_fd, &data->fb0);
> > > > --
> > > > 2.20.1
> > > >
> > > > _______________________________________________
> > > > igt-dev mailing list
> > > > igt-dev at lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/igt-dev
> > >
> > > --
> > > Ville Syrjälä
> > > Intel
>
> --
> Ville Syrjälä
> Intel
More information about the igt-dev
mailing list