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

Rodrigo Siqueira Jordao rjordrigo at amd.com
Wed Nov 24 15:16:59 UTC 2021



On 2021-11-19 10:51 a.m., Juha-Pekka Heikkila wrote:
> On 19.11.2021 16.47, Rodrigo Siqueira Jordao wrote:
>> 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 Siqueira,
> 
> I was not objecting of upstreaming AMD-specific tests, I don't have AMD 
> hw at hand hence I could not guess what are corners there might be hit. 
> I was just pointing there's test which seems to do by code and 
> description mostly what is done here, agreed only linear modifier test 
> will run on AMD hw now.
> 
> I think would be good idea to put TODO comment in to remind about 
> similarity of these tests, if they're the same test someone can go and 
> start merging them later.

Thanks Mark/Juha-Pekka/Lakshmi

I will create a README and a TODO file under amdgpu folder to keep track 
of these suggestions.

Thanks again
Siqueira

> /Juha-Pekka
> 
>>
>> 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