[igt-dev] [PATCH i-g-t 5/5] tests/intel/kms_psr: made test compaitable with pr

Joshi, Kunal1 kunal1.joshi at intel.com
Wed Oct 25 06:19:59 UTC 2023


Hello Jouni,

-----Original Message-----
From: Hogander, Jouni <jouni.hogander at intel.com> 
Sent: Monday, October 23, 2023 12:24 PM
To: Joshi, Kunal1 <kunal1.joshi at intel.com>; igt-dev at lists.freedesktop.org
Cc: Murthy, Arun R <arun.r.murthy at intel.com>; Manna, Animesh <animesh.manna at intel.com>
Subject: Re: [PATCH i-g-t 5/5] tests/intel/kms_psr: made test compaitable with pr

On Fri, 2023-10-20 at 11:35 +0530, Kunal Joshi wrote:
> Modified kms_psr to have support for DP pr.
> 
> Cc: Jouni Högander <jouni.hogander at intel.com>
> Cc: Animesh Manna <animesh.manna at intel.com>
> Cc: Arun R Murthy <arun.r.murthy at intel.com>
> Signed-off-by: Kunal Joshi <kunal1.joshi at intel.com>
> ---
>  tests/intel/kms_psr.c | 402 ++++++++++++++++++++++++++++++++++------
> --
>  1 file changed, 331 insertions(+), 71 deletions(-)
> 
> diff --git a/tests/intel/kms_psr.c b/tests/intel/kms_psr.c index 
> ffecc5222..b14267dce 100644
> --- a/tests/intel/kms_psr.c
> +++ b/tests/intel/kms_psr.c
> @@ -229,6 +229,102 @@
>   * @plane_move:         Move plane position
>   */
>  
> +/**
> + * SUBTEST: pr_dpms
> + * Description: Check if pr is detecting changes when rendering
> operation
> + *              is performed  with dpms enabled or disabled
> + * Driver requirement: i915, xe
> + * Functionality: dpms, pr
> + * Mega feature: Panel Replay
> + * Test category: functionality test
> + *
> + * SUBTEST: pr_no_drrs
> + * Description: Check if pr is detecting changes when drrs is
> disabled
> + * Driver requirement: i915, xe
> + * Functionality: drrs, pr
> + * Mega feature: Panel Replay
> + * Test category: functionality test
> + *
> + * SUBTEST: pr_suspend
> + * Description: Check if pr is detecting changes when plane
> operation is
> + *              performed with suspend resume cycles
> + * Driver requirement: i915, xe
> + * Functionality: pr, suspend
> + * Mega feature: Panel Replay
> + * Test category: functionality test
> + *
> + * SUBTEST: pr_basic
> + * Description: Basic check for pr if it is detecting changes made
> in planes
> + * Driver requirement: i915, xe
> + * Functionality: pr
> + * Mega feature: Panel Replay
> + * Test category: functionality test
> + *
> + * SUBTEST: pr_%s_%s
> + * Description: Check if pr is detecting memory mapping, rendering
> and plane
> + *              operations performed on %arg[1] planes
> + * Driver requirement: i915
> + * Functionality: kms_core, plane, pr
> + * Mega feature: Panel Replay
> + * Test category: functionality test
> + *
> + * arg[1]:
> + *
> + * @cursor:             Cursor plane
> + * @primary:            Primary plane
> + * @sprite:             Sprite plane
> + *
> + * arg[2]:
> + *
> + * @mmap_cpu:           MMAP CPU
> + * @mmap_gtt:           MMAP GTT
> + */
> +
> +/**
> + * SUBTEST: pr_primary_page_flip
> + * Description: Check if pr is detecting memory mapping, rendering
> and plane
> + *              operations performed on primary planes
> + * Driver requirement: i915, xe
> + * Functionality: plane, pr
> + * Mega feature: Panel Replay
> + * Test category: functionality test
> + *
> + * SUBTEST: pr_primary_%s
> + * Description: Check if pr is detecting memory mapping, rendering
> and plane
> + *              operations performed on primary planes
> + * Driver requirement: i915, xe
> + * Functionality: kms_core, plane, pr
> + * Mega feature: Panel Replay
> + * Test category: functionality test
> + *
> + * arg[1]:
> + *
> + * @blt:                Blitter
> + * @render:             Render
> + */
> +
> +/**
> + * SUBTEST: pr_%s_%s
> + * Description: Check if pr is detecting memory mapping, rendering
> and plane
> + *              operations performed on %arg[1] planes
> + * Driver requirement: i915, xe
> + * Functionality: kms_core, plane, pr
> + * Mega feature: Panel Replay
> + * Test category: functionality test
> + *
> + * arg[1]:
> + *
> + * @cursor:             Cursor plane
> + * @sprite:             Sprite plane
> + *
> + * arg[2]:
> + *
> + * @blt:                Blitter
> + * @render:             Render
> + * @plane_onoff:        Plane On off
> + * @plane_move:         Move plane position */
> +
>  enum operations {
>         PAGE_FLIP,
>         MMAP_GTT,
> @@ -272,6 +368,9 @@ typedef struct {
>         igt_output_t *output;
>         bool with_psr_disabled;
>         bool supports_psr2;
> +        bool supports_psr;
> +       bool supports_pr;
> +       igt_output_t **pr_outputs;
>  } data_t;
>  
>  static void create_cursor_fb(data_t *data) @@ -316,7 +415,6 @@ static 
> void setup_output(data_t *data)
>  static void display_init(data_t *data)
>  {
>         igt_display_require(&data->display, data->drm_fd);
> -       setup_output(data);
>  }
>  
>  static void display_fini(data_t *data) @@ -444,6 +542,15 @@ static 
> bool psr_wait_entry_if_enabled(data_t
> *data)
>         return psr_wait_entry(data->debugfs_fd, data->op_psr_mode);
>  }
>  
> +static bool pr_wait_entry(data_t *data) {
> +        if (data->with_psr_disabled)
> +                return true;
> +
> +        return igt_wait(dp_pr_active_check(data->debugfs_fd,
> +                                          data->output), 500, 20); }
> +
>  static bool psr_wait_update_if_enabled(data_t *data)
>  {
>         if (data->with_psr_disabled)
> @@ -604,7 +711,10 @@ static void run_test(data_t *data)
>                 expected = "screen GREEN";
>                 break;
>         }
> -       igt_assert(psr_wait_update_if_enabled(data));
> +       if(strstr(data->output->name, "DP"))
> +               igt_assert(pr_wait_entry(data));
> +       else
> +               igt_assert(psr_wait_update_if_enabled(data));
>         manual(expected);
>  }
>  
> @@ -687,6 +797,7 @@ static void test_setup(data_t *data)
>  {
>         drmModeConnectorPtr connector;
>         bool psr_entered = false;
> +       bool pr_entered = false;
>  
>         igt_require_f(data->output,
>                       "No available output found\n"); @@ -703,15 
> +814,23 @@ static void test_setup(data_t *data)
>                 if (data->op_psr_mode == PSR_MODE_2)
>                         igt_require(data->supports_psr2);
>  
> -               psr_enable_if_enabled(data);
> -               setup_test_plane(data, data->test_plane_id);
> -               if (psr_wait_entry_if_enabled(data)) {
> -                       psr_entered = true;
> -                       break;
> +               if (strstr(data->output->name, "DP"))
> +               {
> +                       setup_test_plane(data, data->test_plane_id);
> +                       pr_entered = pr_wait_entry(data);
> +                       if(pr_entered)
> +                               break;
> +               }
> +               else {
> +                       psr_enable_if_enabled(data);
> +                       setup_test_plane(data, data->test_plane_id);
> +                       if(psr_wait_entry_if_enabled(data)) {
> +                               psr_entered = true;
> +                               break;
> +                       }
>                 }
>         }
> -
> -       igt_assert(psr_entered);
> +       igt_assert(psr_entered || pr_entered);
>  }
>  
>  static void dpms_off_on(data_t *data) @@ -748,10 +867,15 @@ data_t 
> data = {};
>  igt_main_args("", long_options, help_str, opt_handler, &data)
>  {
>         enum operations op;
> -       const char *append_subtest_name[2] = {
> +       const char *append_subtest_name[3] = {
>                 "",
> -               "psr2_"
> +               "psr2_",
> +               "pr_"
>         };
> +       int pr_output_count;
> +       igt_output_t *out;
> +       int modes[] = {PSR_MODE_1, PSR_MODE_1, PR_MODE};

PSR_MODE_1 is listed twice.

> +       int z;
>  
>         igt_fixture {
>                 data.drm_fd = drm_open_driver_master(DRIVER_INTEL | 
> DRIVER_XE); @@ -759,103 +883,239 @@ igt_main_args("", long_options, 
> help_str, opt_handler, &data)
>                 kmstest_set_vt_graphics_mode();
>                 data.devid = intel_get_drm_devid(data.drm_fd);
>  
> -               igt_require_f(sink_support(&data, PSR_MODE_1),
> -                             "Sink does not support PSR\n");
> -
> +               pr_output_count = 0;
> +               data.pr_outputs = (igt_output_t
> **)malloc(sizeof(igt_output_t *) * data.display.n_outputs);
> +               for_each_connected_output(&data.display, out)
> +                       if(output_supports_pr(data.debugfs_fd, out))
> +                               data.pr_outputs[pr_output_count++] =
> out;
> +               data.supports_psr = sink_support(&data, PSR_MODE_1);
>                 data.supports_psr2 = sink_support(&data, PSR_MODE_2);
> +               data.supports_pr = pr_output_count > 0 ? true :
> false;
> +
> +               igt_require_f(data.supports_psr || data.supports_pr,
> +                             "Sink does not support PSR/PR\n");
>                 data.bops = buf_ops_create(data.drm_fd);
>                 display_init(&data);
>         }
>  
> -       for (data.op_psr_mode = PSR_MODE_1; data.op_psr_mode <= 
> PSR_MODE_2;
> -            data.op_psr_mode++) {
> -
> +       for (z = 0; z < ARRAY_SIZE(modes); z++) {
> +               data.op_psr_mode = modes[z];

Why are you changing this? To my opinion original is more clear...

>                 igt_describe("Basic check for psr if it is detecting 
> changes made in planes");
> -               igt_subtest_f("%sbasic",
> append_subtest_name[data.op_psr_mode]) {
> -                       data.test_plane_id = DRM_PLANE_TYPE_PRIMARY;
> -                       test_setup(&data);
> -                       test_cleanup(&data);
> +               igt_subtest_with_dynamic_f("%sbasic",
> append_subtest_name[z]) {
> +                       if (data.op_psr_mode == PR_MODE)
> +                       {
> +                               for(int k = 0; k < pr_output_count;
> k++) {
> +                                       igt_dynamic_f("%s",
> data.pr_outputs[k]->name)
> +                                       {
> +                                               data.output =
> data.pr_outputs[k];
> +                                               data.test_plane_id =
> DRM_PLANE_TYPE_PRIMARY;
> +                                               test_setup(&data);
> +                                               test_cleanup(&data);
> +                                       }
> +                               }
> +                       }
> +                       else {
> +                               setup_output(&data);
> +                               igt_dynamic_f("%s", data.output-
> >name) {
> +                                       data.test_plane_id =
> DRM_PLANE_TYPE_PRIMARY;
> +                                       test_setup(&data);
> +                                       test_cleanup(&data);
> +                               }
> +                       }

> I would suggest using outputs and output_count instead of pr_* and drop setup_output for
> PSR as wll. Then you will have 1 output for PSR and many outpus for PR. This way there is no 
> need to duplicate so much.
> Same comment applies on each test below.
>
> BR,
>
> Jouni Högander

Sure agree, will address in next revision.

>                 }
>  
>                 igt_describe("Check if psr is detecting changes when 
> drrs is disabled");
> -               igt_subtest_f("%sno_drrs",
> append_subtest_name[data.op_psr_mode]) {
> -                       data.test_plane_id = DRM_PLANE_TYPE_PRIMARY;
> -                       test_setup(&data);
> -                       igt_assert(drrs_disabled(&data));
> -                       test_cleanup(&data);
> +               igt_subtest_with_dynamic_f("%sno_drrs",
> append_subtest_name[z]) {
> +                        if (data.op_psr_mode == PR_MODE)
> +                        {
> +                                for(int k = 0; k < pr_output_count;
> k++) {
> +                                        igt_dynamic_f("%s",
> data.pr_outputs[k]->name)
> +                                        {
> +                                               data.output =
> data.pr_outputs[k];
> +                                               data.test_plane_id =
> DRM_PLANE_TYPE_PRIMARY;
> +                                               test_setup(&data);
> +                                               igt_assert(drrs_disab
> led(&data));
> +                                               test_cleanup(&data);
> +
> +                                        }
> +                               }
> +                        }
> +                       else {
> +                               setup_output(&data);
> +                               igt_dynamic_f("%s", data.output-
> >name) {
> +                                       data.test_plane_id =
> DRM_PLANE_TYPE_PRIMARY;
> +                                       test_setup(&data);
> +                                       igt_assert(drrs_disabled(&dat
> a));
> +                                       test_cleanup(&data);
> +                               }
> +                       }
>                 }
>  
>                 for (op = PAGE_FLIP; op <= RENDER; op++) {
>                         igt_describe("Check if psr is detecting page- 
> flipping,memory mapping and "
>                                         "rendering operations 
> performed on primary planes");
> -                       igt_subtest_f("%sprimary_%s",
> -
> append_subtest_name[data.op_psr_mode],
> +                       igt_subtest_with_dynamic_f("%sprimary_%s",
> +                                     append_subtest_name[z],
>                                       op_str(op)) {
>                                 igt_skip_on(is_xe_device(data.drm_fd)
> &&
>                                             (op == MMAP_CPU || op == 
> MMAP_GTT));
> -
> -                               data.op = op;
> -                               data.test_plane_id = 
> DRM_PLANE_TYPE_PRIMARY;
> -                               test_setup(&data);
> -                               run_test(&data);
> -                               test_cleanup(&data);
> +                               if (data.op_psr_mode == PR_MODE)
> +                               {
> +                                       for(int k = 0; k <
> pr_output_count; k++) {
> +                                               igt_dynamic_f("%s",
> data.pr_outputs[k]->name)
> +                                               {
> +                                                       data.output =
> data.pr_outputs[k];
> +                                                       data.op = op;
> +                                                       data.test_pla
> ne_id = DRM_PLANE_TYPE_PRIMARY;
> +                                                       test_setup(&d
> ata);
> +                                                       run_test(&dat
> a);
> +                                                       test_cleanup(
> &data);
> +                                               }
> +                                       }
> +                               }
> +                               else {
> +                                       setup_output(&data);
> +                                       igt_dynamic_f("%s",
> data.output->name) {
> +                                               data.op = op;
> +                                               data.test_plane_id =
> DRM_PLANE_TYPE_PRIMARY;
> +                                               test_setup(&data);
> +                                               run_test(&data);
> +                                               test_cleanup(&data);
> +                                       }
> +                               }
>                         }
>                 }
>  
>                 for (op = MMAP_GTT; op <= PLANE_ONOFF; op++) {
>                         igt_describe("Check if psr is detecting memory 
> mapping,rendering "
>                                         "and plane operations 
> performed on sprite planes");
> -                       igt_subtest_f("%ssprite_%s",
> -
> append_subtest_name[data.op_psr_mode],
> +                       igt_subtest_with_dynamic_f("%ssprite_%s",
> +                                     append_subtest_name[z],
>                                       op_str(op)) {
>                                 igt_skip_on(is_xe_device(data.drm_fd)
> &&
>                                             (op == MMAP_CPU || op == 
> MMAP_GTT));
> -
> -                               data.op = op;
> -                               data.test_plane_id = 
> DRM_PLANE_TYPE_OVERLAY;
> -                               test_setup(&data);
> -                               run_test(&data);
> -                               test_cleanup(&data);
> +                               if (data.op_psr_mode == PR_MODE)
> +                               {
> +                                       for(int k = 0; k <
> pr_output_count; k++) {
> +                                               igt_dynamic_f("%s",
> data.pr_outputs[k]->name)
> +                                               {
> +                                                       data.output =
> data.pr_outputs[k];
> +                                                       data.op = op;
> +                                                       data.test_pla
> ne_id = DRM_PLANE_TYPE_OVERLAY;
> +                                                       test_setup(&d
> ata);
> +                                                       run_test(&dat
> a);
> +                                                       test_cleanup(
> &data);
> +                                               }
> +                                       }
> +                               }
> +                               else {
> +                                       setup_output(&data);
> +                                       igt_dynamic_f("%s",
> data.output->name) {
> +                                               data.op = op;
> +                                               data.test_plane_id =
> DRM_PLANE_TYPE_OVERLAY;
> +                                               test_setup(&data);
> +                                               run_test(&data);
> +                                               test_cleanup(&data);
> +                                       }
> +                               }
>                         }
>  
>                         igt_describe("Check if psr is detecting memory 
> mapping, rendering "
> -                                       "and plane operations 
> performed on cursor planes");
> -                       igt_subtest_f("%scursor_%s",
> -
> append_subtest_name[data.op_psr_mode],
> -                                     op_str(op)) {
> +                                    "and plane operations performed
> on cursor planes");
> +                       igt_subtest_with_dynamic_f("%scursor_%s",
> +                                                 
> append_subtest_name[z],
> +                                                  op_str(op)) {
>                                 igt_skip_on(is_xe_device(data.drm_fd)
> &&
> -                                           (op == MMAP_CPU || op == 
> MMAP_GTT));
> -
> -                               data.op = op;
> -                               data.test_plane_id = 
> DRM_PLANE_TYPE_CURSOR;
> -                               test_setup(&data);
> -                               run_test(&data);
> -                               test_cleanup(&data);
> +                                          (op == MMAP_CPU || op ==
> MMAP_GTT));
> +                               if (data.op_psr_mode == PR_MODE)
> +                               {
> +                                       for(int k = 0; k <
> pr_output_count; k++) {
> +                                               igt_dynamic_f("%s",
> data.pr_outputs[k]->name)
> +                                               {
> +                                                       data.output =
> data.pr_outputs[k];
> +                                                       data.op = op;
> +                                                       data.test_pla
> ne_id = DRM_PLANE_TYPE_CURSOR;
> +                                                       test_setup(&d
> ata);
> +                                                       run_test(&dat
> a);
> +                                                       test_cleanup(
> &data);
> +                                               }
> +                                       }
> +                               }
> +                               else {
> +                                       setup_output(&data);
> +                                       igt_dynamic_f("%s",
> data.output->name) {
> +                                               data.op = op;
> +                                               data.test_plane_id =
> DRM_PLANE_TYPE_CURSOR;
> +                                               test_setup(&data);
> +                                               run_test(&data);
> +                                               test_cleanup(&data);
> +                                       }
> +                               }
>                         }
>                 }
>  
> -               igt_describe("Check if psr is detecting changes when 
> rendering operation is performed"
> -                               "  with dpms enabled or disabled");
> -               igt_subtest_f("%sdpms",
> append_subtest_name[data.op_psr_mode]) {
> -                       data.op = RENDER;
> -                       data.test_plane_id = DRM_PLANE_TYPE_PRIMARY;
> -                       test_setup(&data);
> -                       dpms_off_on(&data);
> -                       run_test(&data);
> -                       test_cleanup(&data);
> +               igt_describe("Check if psr is detecting changes when
> rendering operation is performed "
> +                            "with dpms enabled or disabled");
> +               igt_subtest_with_dynamic_f("%sdpms",
> append_subtest_name[z]) {
> +                       if (data.op_psr_mode == PR_MODE)
> +                       {
> +                               for(int k = 0; k < pr_output_count;
> k++) {
> +                                       igt_dynamic_f("%s",
> data.pr_outputs[k]->name)
> +                                       {
> +                                               data.output =
> data.pr_outputs[k];
> +                                               data.op = RENDER;
> +                                               data.test_plane_id =
> DRM_PLANE_TYPE_PRIMARY;
> +                                               test_setup(&data);
> +                                               dpms_off_on(&data);
> +                                               run_test(&data);
> +                                               test_cleanup(&data);
> +                                       }
> +                               }
> +                       }
> +                       else {
> +                               setup_output(&data);
> +                               igt_dynamic_f("%s", data.output-
> >name) {
> +                                       data.op = RENDER;
> +                                       data.test_plane_id =
> DRM_PLANE_TYPE_PRIMARY;
> +                                       test_setup(&data);
> +                                       dpms_off_on(&data);
> +                                       run_test(&data);
> +                                       test_cleanup(&data);
> +                               }
> +                       }
>                 }
>  
>                 igt_describe("Check if psr is detecting changes when 
> plane operation is performed "
>                                 "with suspend resume cycles");
> -               igt_subtest_f("%ssuspend",
> append_subtest_name[data.op_psr_mode]) {
> -                       data.op = PLANE_ONOFF;
> -                       data.test_plane_id = DRM_PLANE_TYPE_CURSOR;
> -                       test_setup(&data);
> -
>                        igt_system_suspend_autoresume(SUSPEND_STATE_MEM
> ,
> - SUSPEND_TEST_NONE);
> -                       igt_assert(psr_wait_entry_if_enabled(&data));
> -                       run_test(&data);
> -                       test_cleanup(&data);
> +               igt_subtest_with_dynamic_f("%ssuspend",
> append_subtest_name[z]) {
> +                       if (data.op_psr_mode == PR_MODE)
> +                       {
> +                               for(int k = 0; k < pr_output_count;
> k++) {
> +                                       igt_dynamic_f("%s",
> data.pr_outputs[k]->name)
> +                                       {
> +                                               data.output =
> data.pr_outputs[k];
> +                                               data.op =
> PLANE_ONOFF;
> +                                               data.test_plane_id =
> DRM_PLANE_TYPE_CURSOR;
> +                                               test_setup(&data);
> +                                               igt_system_suspend_au
> toresume(SUSPEND_STATE_MEM,
> +                                                                    
>          SUSPEND_TEST_NONE);
> +                                               igt_assert(pr_wait_en
> try(&data));
> +                                               run_test(&data);
> +                                               test_cleanup(&data);
> +                                       }
> +                               }
> +                       }
> +                       else {
> +                               setup_output(&data);
> +                               igt_dynamic_f("%s", data.output-
> >name) {
> +                                       data.op = RENDER;
> +                                       data.test_plane_id =
> DRM_PLANE_TYPE_PRIMARY;
> +                                       test_setup(&data);
> +                                       dpms_off_on(&data);
> +                                       run_test(&data);
> +                                       test_cleanup(&data);
> +                               }
> +                       }
>                 }
>         }
>  
Thanks and Regards
Kunal Joshi


More information about the igt-dev mailing list