[igt-dev] [PATCH i-g-t] amdgpu/amd_mode_switch: Introduce mode switch test

Vudum, Lakshminarayana lakshminarayana.vudum at intel.com
Fri Nov 19 17:25:05 UTC 2021


Re-reported. Here is the bug for the regression
https://gitlab.freedesktop.org/drm/intel/-/issues/4564
igt at gem_exec_suspend@basic-s3 - incomplete - INFO: task jbd2/nvme0n1p2-:161 blocked for more than 30 seconds.

Lakshmi.

-----Original Message-----
From: Rodrigo Siqueira Jordao <rjordrigo at amd.com> 
Sent: Friday, November 19, 2021 6:48 AM
To: Juha-Pekka Heikkilä <juhapekka.heikkila at gmail.com>; Mark Yacoub <markyacoub at chromium.org>; Rodrigo Siqueira <Rodrigo.Siqueira at amd.com>; Vudum, Lakshminarayana <lakshminarayana.vudum at intel.com>
Cc: igt-dev at lists.freedesktop.org; markyacoub at google.com; Wentland, Harry <Harry.Wentland at amd.com>
Subject: Re: [igt-dev] [PATCH i-g-t] amdgpu/amd_mode_switch: Introduce mode switch test

Hi,

On 2021-11-16 5:05 p.m., Juha-Pekka Heikkilä wrote:
> I was wondering how is this different from kms_plane_lowres? On 
> kms_plane_lowres is also used crc to check also what's on screen stay good.

Afaik, we ignore kms_plane_lowres since most tests look related to tiling-[x|y], which I think is an Intel modifier (again, maybe I'm wrong). Anyway, I tried to run it on an AMD device, and most kms_plane_lowres subtest skips.

> /Juha-Pekka
> 
> Mark Yacoub kirjoitti 16.11.2021 klo 23.29:
>> The test looks pretty generic to me, can we make it a kms_ instead of 
>> AMD only?

Mark, Juha-Pekka,

Tbh, right now, I prefer to keep this test as AMD-specific because we are trying to fully upstream all of our tests, so we can rely on igt from the upstream. I can create a README file for amdgpu tests and create a TODO section where I can highlight that we can make this test and other generic; how about that?

Hi Lakshmi,

This test is specific to AMD, but CI is failing. I think it is another false-positive. Here is the patch:

https://patchwork.freedesktop.org/series/96996/

Thanks
Siqueira

>> On Tue, Nov 16, 2021 at 4:00 PM Rodrigo Siqueira 
>> <Rodrigo.Siqueira at amd.com> wrote:
>>>
>>> From: Aurabindo Pillai <aurabindo.pillai at amd.com>
>>>
>>> Switching between different modes could potentially create pstate 
>>> warnings if clocks are not being set correctly.
>>>
>>> The subtest gets the highest and lowest mode supported on the 
>>> connector and switch between them.
>>>
>>> Signed-off-by: Rodrigo Siqueira <Rodrigo.Siqueira at amd.com>
>>> Signed-off-by: Aurabindo Pillai <aurabindo.pillai at amd.com>
>>> ---
>>>   tests/amdgpu/amd_mode_switch.c | 210 
>>> +++++++++++++++++++++++++++++++++
>>>   tests/amdgpu/meson.build       |   1 +
>>>   2 files changed, 211 insertions(+)
>>>   create mode 100644 tests/amdgpu/amd_mode_switch.c
>>>
>>> diff --git a/tests/amdgpu/amd_mode_switch.c 
>>> b/tests/amdgpu/amd_mode_switch.c new file mode 100644 index 
>>> 00000000..270b1b9f
>>> --- /dev/null
>>> +++ b/tests/amdgpu/amd_mode_switch.c
>>> @@ -0,0 +1,210 @@
>>> +/*
>>> + * Copyright 2021 Advanced Micro Devices, Inc.
>>> + *
>>> + * Permission is hereby granted, free of charge, to any person
>>> obtaining a
>>> + * copy of this software and associated documentation files (the
>>> "Software"),
>>> + * to deal in the Software without restriction, including without
>>> limitation
>>> + * the rights to use, copy, modify, merge, publish, distribute,
>>> sublicense,
>>> + * and/or sell copies of the Software, and to permit persons to 
>>> + whom
>>> the
>>> + * Software is furnished to do so, subject to the following conditions:
>>> + *
>>> + * The above copyright notice and this permission notice shall be
>>> included in
>>> + * all copies or substantial portions of the Software.
>>> + *
>>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>>> EXPRESS OR
>>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
>>> MERCHANTABILITY,
>>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO
>>> EVENT SHALL
>>> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM,
>>> DAMAGES OR
>>> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
>>> OTHERWISE,
>>> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
>>> USE OR
>>> + * OTHER DEALINGS IN THE SOFTWARE.
>>> + */
>>> +
>>> +#include "igt.h"
>>> +#include <xf86drmMode.h>
>>> +
>>> +#define MAX_PIPES 6
>>> +
>>> +/* Common test data. */
>>> +typedef struct data {
>>> +       igt_display_t display;
>>> +       igt_plane_t *primary[MAX_PIPES];
>>> +       igt_output_t *output[MAX_PIPES];
>>> +       int fd;
>>> +} data_t;
>>> +
>>> +static void test_init(data_t *data) {
>>> +       igt_display_t *display = &data->display;
>>> +       int i;
>>> +
>>> +       for_each_pipe(display, i) {
>>> +               igt_output_t *output = &display->outputs[i];
>>> +
>>> +               data->primary[i] = igt_pipe_get_plane_type(
>>> +                       &data->display.pipes[i],
>>> DRM_PLANE_TYPE_PRIMARY);
>>> +
>>> +               data->output[i] = output;
>>> +       }
>>> +
>>> +       igt_require(data->output[0]);
>>> +       igt_display_reset(display);
>>> +}
>>> +
>>> +static void test_fini(data_t *data) {
>>> +       igt_display_t *display = &data->display;
>>> +
>>> +       igt_display_reset(display);
>>> +       igt_display_commit_atomic(display,
>>> DRM_MODE_ATOMIC_ALLOW_MODESET, 0);
>>> +}
>>> +
>>> +/* Forces a mode for a connector. */ static void 
>>> +force_output_mode(data_t *d, igt_output_t *output,
>>> +                             const drmModeModeInfo *mode) {
>>> +       /* This allows us to create a virtual sink. */
>>> +       if (!igt_output_is_connected(output)) {
>>> +               kmstest_force_edid(d->fd, output->config.connector,
>>> +                                  igt_kms_get_4k_edid());
>>> +
>>> +               kmstest_force_connector(d->fd, 
>>> +output->config.connector,
>>> +                                       FORCE_CONNECTOR_DIGITAL);
>>> +       }
>>> +
>>> +       igt_output_override_mode(output, mode); }
>>> +
>>> +static void run_mode_switch_first_last(data_t *data, int num_pipes) 
>>> +{
>>> +       igt_output_t *output;
>>> +       struct igt_fb *buffer1[MAX_PIPES] = { NULL };
>>> +       struct igt_fb *buffer2[MAX_PIPES] = { NULL };
>>> +       drmModeConnectorPtr conn;
>>> +       drmModeModeInfoPtr kmode;
>>> +       void *user_data = NULL;
>>> +       int i = 0;
>>> +       int j = 0;
>>> +
>>> +       test_init(data);
>>> +
>>> +       igt_skip_on_f(num_pipes >
>>> igt_display_get_n_pipes(&data->display) ||
>>> +                             num_pipes > data->display.n_outputs,
>>> +                     "ASIC does not have %d outputs/pipes\n",
>>> num_pipes);
>>> +
>>> +       /* First supported mode */
>>> +
>>> +       for (j = 0; j < num_pipes; j++) {
>>> +               output = data->output[j];
>>> +               if (!igt_output_is_connected(output))
>>> +                       continue;
>>> +
>>> +               conn = drmModeGetConnector(
>>> +                       data->fd,
>>> output->config.connector->connector_id);
>>> +
>>> +               kmode = &conn->modes[0];
>>> +               if (buffer1[j] == NULL) {
>>> +                       igt_fb_t fb;
>>> +                       buffer1[j] = &fb;
>>> +                       igt_create_color_fb(data->fd, 
>>> +kmode->hdisplay,
>>> +                                           kmode->vdisplay,
>>> +                                           DRM_FORMAT_XRGB8888,
>>> +                                           DRM_FORMAT_MOD_NONE, 
>>> +1.f,
>>> 0.f,
>>> +                                           0.f, buffer1[j]);
>>> +               }
>>> +               igt_output_set_pipe(output, j);
>>> +               force_output_mode(data, output, kmode);
>>> +               igt_plane_set_fb(data->primary[j], buffer1[j]);
>>> +               drmModeFreeConnector(conn);
>>> +       }
>>> +
>>> +       igt_display_commit_atomic(&data->display,
>>> DRM_MODE_ATOMIC_ALLOW_MODESET,
>>> +                                 user_data);
>>> +
>>> +       /* Last supported mode */
>>> +
>>> +       for (j = 0; j < num_pipes; j++) {
>>> +               output = data->output[j];
>>> +               if (!igt_output_is_connected(output))
>>> +                       continue;
>>> +
>>> +               conn = drmModeGetConnector(
>>> +                       data->fd,
>>> output->config.connector->connector_id);
>>> +
>>> +               kmode = &conn->modes[conn->count_modes - 1];
>>> +               if (buffer2[j] == NULL) {
>>> +                       igt_fb_t fb;
>>> +                       buffer2[j] = &fb;
>>> +                       igt_create_color_fb(data->fd, 
>>> +kmode->hdisplay,
>>> +                                           kmode->vdisplay,
>>> +                                           DRM_FORMAT_XRGB8888,
>>> +                                           DRM_FORMAT_MOD_NONE, 
>>> +1.f,
>>> 0.f,
>>> +                                           0.f, buffer2[j]);
>>> +               }
>>> +               force_output_mode(data, output, kmode);
>>> +               igt_plane_set_fb(data->primary[j], buffer2[j]);
>>> +               drmModeFreeConnector(conn);
>>> +       }
>>> +
>>> +       igt_display_commit_atomic(&data->display,
>>> DRM_MODE_ATOMIC_ALLOW_MODESET,
>>> +                                 user_data);
>>> +
>>> +       /* First supported again */
>>> +       for (j = 0; j < num_pipes; j++) {
>>> +               output = data->output[j];
>>> +               if (!igt_output_is_connected(output))
>>> +                       continue;
>>> +
>>> +               conn = drmModeGetConnector(
>>> +                       data->fd,
>>> output->config.connector->connector_id);
>>> +
>>> +               kmode = &conn->modes[0];
>>> +               force_output_mode(data, output, kmode);
>>> +               igt_plane_set_fb(data->primary[j], buffer1[j]);
>>> +               drmModeFreeConnector(conn);
>>> +       }
>>> +
>>> +       igt_display_commit_atomic(&data->display,
>>> DRM_MODE_ATOMIC_ALLOW_MODESET,
>>> +                                 user_data);
>>> +
>>> +       test_fini(data);
>>> +
>>> +       for (i = 0; i <= num_pipes; i++) {
>>> +               igt_remove_fb(data->fd, buffer1[i]);
>>> +               igt_remove_fb(data->fd, buffer2[i]);
>>> +       }
>>> +}
>>> +
>>> +IGT_TEST_DESCRIPTION("Test switching between supported modes"); 
>>> +igt_main {
>>> +       data_t data;
>>> +       int i = 0;
>>> +
>>> +       igt_skip_on_simulation();
>>> +
>>> +       memset(&data, 0, sizeof(data));
>>> +
>>> +       igt_fixture
>>> +       {
>>> +               data.fd = drm_open_driver_master(DRIVER_AMDGPU);
>>> +
>>> +               kmstest_set_vt_graphics_mode();
>>> +
>>> +               igt_display_require(&data.display, data.fd);
>>> +               igt_require(&data.display.is_atomic);
>>> +               igt_display_require_output(&data.display);
>>> +       }
>>> +
>>> +       for (i = 0; i < MAX_PIPES; i++) {
>>> +               igt_describe(
>>> +                       "Test between switching highest and lowest
>>> supported mode");
>>> +               igt_subtest_f("mode-switch-first-last-pipe-%d", i)
>>> +                       run_mode_switch_first_last(&data, i + 1);
>>> +       }
>>> +
>>> +       igt_fixture
>>> +       {
>>> +               igt_display_fini(&data.display);
>>> +       }
>>> +}
>>> diff --git a/tests/amdgpu/meson.build b/tests/amdgpu/meson.build 
>>> index b736c456..5216e194 100644
>>> --- a/tests/amdgpu/meson.build
>>> +++ b/tests/amdgpu/meson.build
>>> @@ -16,6 +16,7 @@ if libdrm_amdgpu.found()
>>>                            'amd_mem_leak',
>>>                            'amd_link_settings',
>>>                            'amd_vrr_range',
>>> +                         'amd_mode_switch',
>>>                          ]
>>>          amdgpu_deps += libdrm_amdgpu
>>>   endif
>>> --
>>> 2.25.1
>>>



More information about the igt-dev mailing list