[igt-dev] [v5 i-g-t] tests/kms_busy: Limit the execution to two pipes
Gupta, Anshuman
anshuman.gupta at intel.com
Wed May 26 08:05:23 UTC 2021
> -----Original Message-----
> From: Modem, Bhanuprakash <bhanuprakash.modem at intel.com>
> Sent: Monday, May 24, 2021 2:22 PM
> To: Gupta, Anshuman <anshuman.gupta at intel.com>; igt-
> dev at lists.freedesktop.org
> Cc: Joshi, Kunal1 <kunal1.joshi at intel.com>; Latvala, Petri
> <petri.latvala at intel.com>; Kunche, Kishore <kishore.kunche at intel.com>;
> Shankar, Uma <uma.shankar at intel.com>; B S, Karthik
> <karthik.b.s at intel.com>
> Subject: RE: [igt-dev] [v5 i-g-t] tests/kms_busy: Limit the execution to two
> pipes
>
> > From: Gupta, Anshuman <anshuman.gupta at intel.com>
> > Sent: Monday, May 24, 2021 10:24 AM
> > To: Modem, Bhanuprakash <bhanuprakash.modem at intel.com>
> > Cc: igt-dev at lists.freedesktop.org; Joshi, Kunal1
> > <kunal1.joshi at intel.com>; Latvala, Petri <petri.latvala at intel.com>;
> > Kunche, Kishore <kishore.kunche at intel.com>
> > Subject: Re: [igt-dev] [v5 i-g-t] tests/kms_busy: Limit the execution
> > to two pipes
> >
> > On 2021-04-12 at 22:27:51 +0530, Bhanuprakash Modem wrote:
> > > As all pipes are symmetric, restrict the execution to two pipes can
> > > save lot of CI time.
> > Above statement is not correct, all pipes are not symmetric, AFAIK
> > PIPE_A has lower power capability with DMC save/restore (low power
> > single pipe) Which is even extend to PIPE_B on XE_LPD, which adds DMC
> > save/restore even for PIPE_B as well (low power dual pipe).
> >
> > please mention the property of each pipe in commit message, so it can
> > be known, we should not loose any validation coverage in order to save
> > CI time.
> >
> > This patch doesn't choose distinct crtc's to run the test.
>
> Actually, we are not touching any crtc properties here, these tests will
> perform "page flip" (or) "modeset" by injecting the HANG.
>
> I am not sure that pipe's power capabilities effects flip/modeset. If it effects,
> we must find a way to identify distinct CRTCs, else restricting the execution
> to pipe A & B seems fine to me.
Above mentioned pipe properties related to power was known to me,
There can be other distinct pipe properties as well.
We can can't overlook any pipe power capabilities, and assume it won't impact flip or modeset.
We had observed underruns from flips/modeset related test in past.
In my opinion there is no harm to execute these test on distinct crtc.
Thanks,
Anshuman.
>
> @Anshuman, Please suggest.
>
> And, sure I'll update the commit message in next rev.
>
> - Bhanu
>
> >
> > Thanks,
> > Anshuman Gupta.
> > >
> > > If we want to execute on all pipes, we need to pass an extra
> > > argument "-e" indicates extended.
> > >
> > > Example: ./build/tests/kms_busy -e --r basic
> > >
> > > V2, V3:
> > > * Fix the typo in args handler (Petri)
> > > V4:
> > > * Handle clampling cleanly (Petri)
> > > * Restore lost test coverage in the previous version (Petri/Bhanu)
> > > V5:
> > > * Fix two subtest with the name "basic" (Petri)
> > >
> > > Cc: Karthik B S <karthik.b.s at intel.com>
> > > Cc: Latvala Petri <petri.latvala at intel.com>
> > > Cc: Kunche Kishore <kishore.kunche at intel.com>
> > > Cc: Uma Shankar <uma.shankar at intel.com>
> > > Reviewed-by: Kunal Joshi <kunal1.joshi at intel.com> (V3)
> > > Signed-off-by: Bhanuprakash Modem
> <bhanuprakash.modem at intel.com>
> > > ---
> > > tests/kms_busy.c | 139
> > > ++++++++++++++++++++++++++++++-----------------
> > > 1 file changed, 89 insertions(+), 50 deletions(-)
> > >
> > > diff --git a/tests/kms_busy.c b/tests/kms_busy.c index
> > > df1f8e11a..095896db5 100644
> > > --- a/tests/kms_busy.c
> > > +++ b/tests/kms_busy.c
> > > @@ -30,6 +30,11 @@
> > >
> > > IGT_TEST_DESCRIPTION("Basic check of KMS ABI with busy
> > > framebuffers.");
> > >
> > > +/* restricted pipe count */
> > > +#define CRTC_RESTRICT_CNT 2
> > > +
> > > +static bool all_pipes = false;
> > > +
> > > static igt_output_t *
> > > set_fb_on_crtc(igt_display_t *dpy, int pipe, struct igt_fb *fb) {
> > > @@ -287,10 +292,41 @@ static void
> > > test_pageflip_modeset_hang(igt_display_t
> > *dpy, enum pipe pipe)
> > > igt_remove_fb(dpy->drm_fd, &fb);
> > > }
> > >
> > > -igt_main
> > > +static int opt_handler(int opt, int opt_index, void *data) {
> > > + switch (opt) {
> > > + case 'e':
> > > + all_pipes = true;
> > > + break;
> > > + default:
> > > + return IGT_OPT_HANDLER_ERROR;
> > > + }
> > > +
> > > + return IGT_OPT_HANDLER_SUCCESS;
> > > +}
> > > +
> > > +const char *help_str =
> > > + " -e \tRun on all pipes. (By default subtests will run on two
> > pipes)\n";
> > > +
> > > +igt_main_args("e", NULL, help_str, opt_handler, NULL)
> > > {
> > > igt_display_t display = { .drm_fd = -1, .n_pipes = IGT_MAX_PIPES };
> > > - enum pipe n;
> > > +
> > > + int crtc_count;
> > > + int i;
> > > + struct {
> > > + const char *name;
> > > + bool modeset;
> > > + bool hang_newfb;
> > > + bool reset;
> > > + } tests[] = {
> > > + { "extended-pageflip-hang-oldfb", false, false, false },
> > > + { "extended-pageflip-hang-newfb", false, true, false },
> > > + { "extended-modeset-hang-oldfb", true, false, false },
> > > + { "extended-modeset-hang-newfb", true, true, false },
> > > + { "extended-modeset-hang-oldfb-with-reset", true, false, true
> },
> > > + { "extended-modeset-hang-newfb-with-reset", true, true,
> true },
> > > + };
> > >
> > > igt_fixture {
> > > int fd = drm_open_driver_master(DRIVER_INTEL);
> > > @@ -318,70 +354,73 @@ igt_main
> > > }
> > > }
> > >
> > > - for_each_pipe_static(n) igt_subtest_group {
> > > - igt_hang_t hang;
> > > -
> > > + igt_subtest_with_dynamic("basic-hang") {
> > > + enum pipe pipe;
> > > + igt_output_t *output;
> > > + int count = 0;
> > > + igt_hang_t hang = igt_allow_hang(display.drm_fd, 0, 0);
> > > errno = 0;
> > >
> > > - igt_fixture {
> > > - igt_display_require_output_on_pipe(&display, n);
> > > - }
> > > -
> > > - igt_subtest_f("basic-flip-pipe-%s", kmstest_pipe_name(n)) {
> > > - test_flip(&display, n, false);
> > > - }
> > > - igt_subtest_f("basic-modeset-pipe-%s",
> kmstest_pipe_name(n)) {
> > > -
> > > - test_flip(&display, n, true);
> > > - }
> > > -
> > > - igt_fixture {
> > > - hang = igt_allow_hang(display.drm_fd, 0, 0);
> > > - }
> > > + crtc_count = (all_pipes)? display.n_pipes :
> CRTC_RESTRICT_CNT;
> > > + for_each_pipe_with_valid_output(&display, pipe, output) {
> > > + igt_dynamic_f("flip-pipe-%s",
> kmstest_pipe_name(pipe))
> > > + test_flip(&display, pipe, false);
> > > + igt_dynamic_f("modeset-pipe-%s",
> kmstest_pipe_name(pipe))
> > > + test_flip(&display, pipe, true);
> > >
> > > - igt_subtest_f("extended-pageflip-modeset-hang-oldfb-pipe-
> %s",
> > > - kmstest_pipe_name(n)) {
> > > - test_pageflip_modeset_hang(&display, n);
> > > + if (++count >= crtc_count)
> > > + break;
> > > }
> > >
> > > - igt_fixture
> > > - igt_require(display.is_atomic);
> > > + igt_disallow_hang(display.drm_fd, hang);
> > > + }
> > >
> > > - igt_subtest_f("extended-pageflip-hang-oldfb-pipe-%s",
> > > - kmstest_pipe_name(n))
> > > - test_hang(&display, n, false, false);
> > > + igt_subtest_with_dynamic("extended-pageflip-modeset-hang-oldfb")
> {
> > > + enum pipe pipe;
> > > + igt_output_t *output;
> > > + int count = 0;
> > > + igt_hang_t hang = igt_allow_hang(display.drm_fd, 0, 0);
> > > + errno = 0;
> > >
> > > - igt_subtest_f("extended-pageflip-hang-newfb-pipe-%s",
> > > - kmstest_pipe_name(n))
> > > - test_hang(&display, n, false, true);
> > > + crtc_count = (all_pipes)? display.n_pipes :
> CRTC_RESTRICT_CNT;
> > > + for_each_pipe_with_valid_output(&display, pipe, output) {
> > > + igt_dynamic_f("pipe-%s", kmstest_pipe_name(pipe))
> > > + test_pageflip_modeset_hang(&display, pipe);
> > >
> > > - igt_subtest_f("extended-modeset-hang-oldfb-pipe-%s",
> > > - kmstest_pipe_name(n))
> > > - test_hang(&display, n, true, false);
> > > + if (++count >= crtc_count)
> > > + break;
> > > + }
> > >
> > > - igt_subtest_f("extended-modeset-hang-newfb-pipe-%s",
> > > - kmstest_pipe_name(n))
> > > - test_hang(&display, n, true, true);
> > > + igt_disallow_hang(display.drm_fd, hang);
> > > + }
> > >
> > > - igt_subtest_f("extended-modeset-hang-oldfb-with-reset-
> pipe-%s",
> > > - kmstest_pipe_name(n)) {
> > > - igt_set_module_param_int(display.drm_fd,
> > "force_reset_modeset_test", 1);
> > > + for (i = 0; i < sizeof(tests) / sizeof (tests[0]); i++) {
> > > + igt_subtest_with_dynamic(tests[i].name) {
> > > + enum pipe pipe;
> > > + igt_output_t *output;
> > > + int count = 0;
> > > + igt_hang_t hang;
> > > + errno = 0;
> > >
> > > - test_hang(&display, n, true, false);
> > > + igt_require(display.is_atomic);
> > > + hang = igt_allow_hang(display.drm_fd, 0, 0);
> > >
> > > - igt_set_module_param_int(display.drm_fd,
> > "force_reset_modeset_test", 0);
> > > - }
> > > + crtc_count = (all_pipes)? display.n_pipes :
> > CRTC_RESTRICT_CNT;
> > > + for_each_pipe_with_valid_output(&display, pipe,
> output) {
> > > + igt_dynamic_f("pipe-%s",
> kmstest_pipe_name(pipe)) {
> > > + if (tests[i].reset)
> > > +
> igt_set_module_param_int(display.drm_fd,
> > "force_reset_modeset_test", 1);
> > >
> > > - igt_subtest_f("extended-modeset-hang-newfb-with-reset-
> pipe-%s",
> > > - kmstest_pipe_name(n)) {
> > > - igt_set_module_param_int(display.drm_fd,
> > "force_reset_modeset_test", 1);
> > > + test_hang(&display, pipe,
> tests[i].modeset,
> > tests[i].hang_newfb);
> > >
> > > - test_hang(&display, n, true, true);
> > > + if (tests[i].reset)
> > > +
> igt_set_module_param_int(display.drm_fd,
> > "force_reset_modeset_test", 0);
> > > + }
> > >
> > > - igt_set_module_param_int(display.drm_fd,
> > "force_reset_modeset_test", 0);
> > > - }
> > > + if (++count >= crtc_count)
> > > + break;
> > > + }
> > >
> > > - igt_fixture {
> > > igt_disallow_hang(display.drm_fd, hang);
> > > }
> > > }
> > > --
> > > 2.20.1
> > >
> > > _______________________________________________
> > > igt-dev mailing list
> > > igt-dev at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/igt-dev
More information about the igt-dev
mailing list