Re: [PATCH i-g-t] tests/intel/kms_pm_backlight: Brightness test during DPMS on and off

["Hogander, Jouni" <jouni.hogander@intel.com>]

On Thu, 2024-07-04 at 17:01 +0530, Mohammed Thasleem wrote:
> Set brightness by deviding max brightness, store and read it back.
> The actual brightness should be same during DPMS on and off cycle.
> 
> v2: Update test path and testplan documentation.
> v3: Check backlight for all internal panels. (Jani)
> 
> Signed-off-by: Mohammed Thasleem <mohammed.thasleem@intel.com>
> ---
>  tests/intel/kms_pm_backlight.c | 51 ++++++++++++++++++++++++++++++--
> --
>  1 file changed, 46 insertions(+), 5 deletions(-)
> 
> diff --git a/tests/intel/kms_pm_backlight.c
> b/tests/intel/kms_pm_backlight.c
> index 8672afa7a..bb85c6e29 100644
> --- a/tests/intel/kms_pm_backlight.c
> +++ b/tests/intel/kms_pm_backlight.c
> @@ -64,6 +64,10 @@
>   * SUBTEST: fade-with-suspend
>   * Description: Test the fade with suspend.
>   * Functionality: backlight, suspend
> + *
> + * SUBTEST: brightness-with-dpms
> + * Description: test brightness with dpms on and off cycle.
> + * Functionality: backlight, backlight
>   */
>  
>  struct context {
> @@ -217,6 +221,32 @@ check_suspend(igt_output_t *output)
>         igt_system_suspend_autoresume(SUSPEND_STATE_MEM,
> SUSPEND_TEST_NONE);
>  }
>  
> +static void check_dpms_cycle(igt_display_t *display, igt_output_t
> *output, struct context *context)
> +{
> +       int max, val_1, val_2;
> +
> +       backlight_read(&max, "max_brightness", context);
> +       igt_assert(max);
> +
> +       backlight_write(max / 2, "brightness", context);
> +       backlight_read(&val_1, "actual_brightness", context);
> +
> +       igt_require(igt_setup_runtime_pm(output->display->drm_fd));
> +
> +       kmstest_set_connector_dpms(output->display->drm_fd,
> +                                  output->config.connector,
> +                                  DRM_MODE_DPMS_OFF);
> +       igt_require(igt_wait_for_pm_status(IGT_RUNTIME_PM_STATUS_SUSP
> ENDED));
> +
> +       kmstest_set_connector_dpms(output->display->drm_fd,
> +                                  output->config.connector,
> +                                  DRM_MODE_DPMS_ON);
> +       igt_assert(igt_wait_for_pm_status(IGT_RUNTIME_PM_STATUS_ACTIV
> E));
> +
> +       backlight_read(&val_2, "actual_brightness", context);
> +       igt_assert_eq(val_1, val_2);
> +}
> +
>  static void test_cleanup(igt_display_t *display, igt_output_t
> *output)
>  {
>         igt_output_set_pipe(output, PIPE_NONE);
> @@ -262,7 +292,7 @@ igt_main
>         igt_display_t display;
>         igt_output_t *output;
>         char file_path_n[PATH_MAX] = "";
> -       bool dual_edp = false;
> +       bool num_panel = false;

I think this is unrelated change and doesn't improve readability:
num_panel is somehow stating it's number of panels, but it is actually
a boolean.

BR,

Jouni Högander

>         struct context contexts[NUM_EDP_OUTPUTS];
>         struct {
>                 const char *name;
> @@ -290,7 +320,7 @@ igt_main
>                 igt_display_require(&display,
> drm_open_driver(DRIVER_INTEL | DRIVER_XE));
>  
>                 for_each_connected_output(&display, output) {
> -                       if (output->config.connector->connector_type
> != DRM_MODE_CONNECTOR_eDP)
> +                       if (!output_is_internal_panel(output))
>                                 continue;
>  
>                         if (found)
> @@ -327,7 +357,7 @@ igt_main
>                                 contexts[i++].output = output;
>  
>                                 if (found)
> -                                       dual_edp = true;
> +                                       num_panel = true;
>                                 else
>                                         found = true;
>                         }
> @@ -338,7 +368,7 @@ igt_main
>         for (i = 0; i < ARRAY_SIZE(tests); i++) {
>                 igt_describe(tests[i].desc);
>                 igt_subtest_with_dynamic(tests[i].name) {
> -                       for (int j = 0; j < (dual_edp ? 2 : 1); j++)
> {
> +                       for (int j = 0; j < (num_panel ? 2 : 1); j++)
> {
>                                 test_setup(display, &contexts-
> >output[j]);
>  
>                                 if (tests[i].flags == TEST_DPMS)
> @@ -355,9 +385,20 @@ igt_main
>                 }
>         }
>  
> +       igt_describe("test brightness with dpms on and off cycle");
> +       igt_subtest_with_dynamic("brightness-with-dpms") {
> +               for (int j = 0; j < (num_panel ? 2 : 1); j++) {
> +                       test_setup(display, &contexts->output[j]);
> +                       igt_dynamic_f("%s",
> igt_output_name(contexts[j].output)) {
> +                               check_dpms_cycle(&display, &contexts-
> >output[j], &contexts[j]);
> +                               test_cleanup(&display, &contexts-
> >output[j]);
> +                       }
> +               }
> +       }
> +
>         igt_fixture {
>                 /* Restore old brightness */
> -               for (i = 0; i < (dual_edp ? 2 : 1); i++)
> +               for (i = 0; i < (num_panel ? 2 : 1); i++)
>                         backlight_write(contexts[i].old,
> "brightness", &contexts[i]);
>  
>                 igt_display_fini(&display);