[igt-dev] [PATCH 4/7] tests/kms_hdr: Add bpc switch subtests
Kazlauskas, Nicholas
nicholas.kazlauskas at amd.com
Mon Feb 3 13:25:40 UTC 2020
Drawing on 10-bit framebuffers in IGT is incredibly slow, especially for
buffers larger than 1080p. I think it was a few seconds of overhead per
test.
AMD hardware can handle unscaled vs scaled without issue so changing it
to the full resolution is fine by me if it helps maintain compatibility
across vendors.
Regards,
Nicholas Kazlauskas
On 2020-02-03 3:49 a.m., Sharma, Swati2 wrote:
> Hi Nicholas,
>
> With this patch we are seeing regression on HSW.
> Earlier we were getting scaler issue, I tried fixing it by having plane
> size to 512x512.
> Now, in dmesg we can see "Plane must cover entire CRTC". For this to fix
> i need to change the plane size to CRTC size; which will give scaler
> issue as earlier. Ultimately i have to change FB size to mode->w, mode->h.
> But according to comment its written "10-bit formats are slow, so limit
> the size"; so what you think is the right way to proceed? Should i
> change FB size? How much slowness is "slow" here?
>
> @Ville what you think?
>
> On 29-Jan-20 7:25 PM, Swati Sharma wrote:
>> From: Nicholas Kazlauskas <nicholas.kazlauskas at amd.com>
>>
>> Add subtests to validate transitions between 8bpc and 10bpc. Test is
>> made compatible for both amd and intel drivers. The test requires
>> the "output_bpc" debugfs entry to read current and maximum bpc.
>> This is exposed by amd driver, however intel driver doesn't expose it,
>> so made that restriction to assert output bpc only for amd driver.
>>
>> v2: -Limiting plane size to 512x512 since regression observed in hsw
>> because of plane scaling restriction.
>> -Minor alignment fix [Uma]
>> v3: -Small fix
>>
>> Cc: Harry Wentland <harry.wentland at amd.com>
>> Cc: Leo Li <sunpeng.li at amd.com>
>> Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas at amd.com>
>> Signed-off-by: Swati Sharma <swati2.sharma at intel.com>
>> Reviewed-by: Uma Shankar <uma.shankar at intel.com>
>> ---
>> tests/Makefile.sources | 1 +
>> tests/kms_hdr.c | 295 +++++++++++++++++++++++++++++++++++++++++
>> tests/meson.build | 1 +
>> 3 files changed, 297 insertions(+)
>> create mode 100644 tests/kms_hdr.c
>>
>> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
>> index 4582f656..98411172 100644
>> --- a/tests/Makefile.sources
>> +++ b/tests/Makefile.sources
>> @@ -54,6 +54,7 @@ TESTS_progs = \
>> kms_frontbuffer_tracking \
>> kms_getfb \
>> kms_hdmi_inject \
>> + kms_hdr \
>> kms_invalid_dotclock \
>> kms_lease \
>> kms_legacy_colorkey \
>> diff --git a/tests/kms_hdr.c b/tests/kms_hdr.c
>> new file mode 100644
>> index 00000000..ff21335a
>> --- /dev/null
>> +++ b/tests/kms_hdr.c
>> @@ -0,0 +1,295 @@
>> +/*
>> + * Copyright 2019 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 <fcntl.h>
>> +#include <termios.h>
>> +#include <unistd.h>
>> +
>> +IGT_TEST_DESCRIPTION("Test HDR metadata interfaces and bpc switch");
>> +
>> +/* Test flags. */
>> +enum {
>> + TEST_NONE = 1 << 0,
>> + TEST_DPMS = 1 << 1,
>> + TEST_SUSPEND = 1 << 2,
>> +};
>> +
>> +/* BPC connector state. */
>> +typedef struct output_bpc {
>> + unsigned int current;
>> + unsigned int maximum;
>> +} output_bpc_t;
>> +
>> +/* Common test data. */
>> +typedef struct data {
>> + igt_display_t display;
>> + igt_plane_t *primary;
>> + igt_output_t *output;
>> + igt_pipe_t *pipe;
>> + igt_pipe_crc_t *pipe_crc;
>> + drmModeModeInfo *mode;
>> + enum pipe pipe_id;
>> + int fd;
>> + int w;
>> + int h;
>> +} data_t;
>> +
>> +/* Common test cleanup. */
>> +static void test_fini(data_t *data)
>> +{
>> + igt_pipe_crc_free(data->pipe_crc);
>> + igt_display_reset(&data->display);
>> +}
>> +
>> +static void test_cycle_flags(data_t *data, uint32_t test_flags)
>> +{
>> + if (test_flags & TEST_DPMS) {
>> + kmstest_set_connector_dpms(data->fd,
>> + data->output->config.connector,
>> + DRM_MODE_DPMS_OFF);
>> + kmstest_set_connector_dpms(data->fd,
>> + data->output->config.connector,
>> + DRM_MODE_DPMS_ON);
>> + }
>> +
>> + if (test_flags & TEST_SUSPEND)
>> + igt_system_suspend_autoresume(SUSPEND_STATE_MEM,
>> + SUSPEND_TEST_NONE);
>> +}
>> +
>> +/* Returns the current and maximum bpc from the connector debugfs. */
>> +static output_bpc_t get_output_bpc(data_t *data)
>> +{
>> + char buf[256];
>> + char *start_loc;
>> + int fd, res;
>> + output_bpc_t info;
>> +
>> + fd = igt_debugfs_connector_dir(data->fd, data->output->name,
>> O_RDONLY);
>> + igt_assert(fd >= 0);
>> +
>> + res = igt_debugfs_simple_read(fd, "output_bpc", buf, sizeof(buf));
>> +
>> + igt_require(res > 0);
>> +
>> + close(fd);
>> +
>> + igt_assert(start_loc = strstr(buf, "Current: "));
>> + igt_assert_eq(sscanf(start_loc, "Current: %u", &info.current), 1);
>> +
>> + igt_assert(start_loc = strstr(buf, "Maximum: "));
>> + igt_assert_eq(sscanf(start_loc, "Maximum: %u", &info.maximum), 1);
>> +
>> + return info;
>> +}
>> +
>> +/* Verifies that connector has the correct output bpc. */
>> +static void assert_output_bpc(data_t *data, unsigned int bpc)
>> +{
>> + output_bpc_t info = get_output_bpc(data);
>> +
>> + igt_require_f(info.maximum >= bpc,
>> + "Monitor doesn't support %u bpc, max is %u\n", bpc,
>> + info.maximum);
>> +
>> + igt_assert_eq(info.current, bpc);
>> +}
>> +
>> +/* Fills the FB with a test HDR pattern. */
>> +static void draw_hdr_pattern(igt_fb_t *fb)
>> +{
>> + cairo_t *cr = igt_get_cairo_ctx(fb->fd, fb);
>> +
>> + igt_paint_color(cr, 0, 0, fb->width, fb->height, 1.0, 1.0, 1.0);
>> + igt_paint_test_pattern(cr, fb->width, fb->height);
>> +
>> + igt_put_cairo_ctx(fb->fd, fb, cr);
>> +}
>> +
>> +/* Prepare test data. */
>> +static void prepare_test(data_t *data, igt_output_t *output, enum
>> pipe pipe)
>> +{
>> + igt_display_t *display = &data->display;
>> +
>> + data->pipe_id = pipe;
>> + data->pipe = &data->display.pipes[data->pipe_id];
>> + igt_assert(data->pipe);
>> +
>> + igt_display_reset(display);
>> +
>> + data->output = output;
>> + igt_assert(data->output);
>> +
>> + data->mode = igt_output_get_mode(data->output);
>> + igt_assert(data->mode);
>> +
>> + data->primary =
>> + igt_pipe_get_plane_type(data->pipe, DRM_PLANE_TYPE_PRIMARY);
>> +
>> + data->pipe_crc = igt_pipe_crc_new(data->fd, data->pipe_id,
>> + INTEL_PIPE_CRC_SOURCE_AUTO);
>> +
>> + igt_output_set_pipe(data->output, data->pipe_id);
>> +
>> + data->w = data->mode->hdisplay;
>> + data->h = data->mode->vdisplay;
>> +}
>> +
>> +static bool igt_pipe_is_free(igt_display_t *display, enum pipe pipe)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < display->n_outputs; i++)
>> + if (display->outputs[i].pending_pipe == pipe)
>> + return false;
>> +
>> + return true;
>> +}
>> +
>> +static void test_bpc_switch_on_output(data_t *data, igt_output_t
>> *output,
>> + uint32_t flags)
>> +{
>> + igt_display_t *display = &data->display;
>> + igt_crc_t ref_crc, new_crc;
>> + enum pipe pipe;
>> + igt_fb_t afb;
>> + int afb_id;
>> +
>> + for_each_pipe(display, pipe) {
>> + if (!igt_pipe_connector_valid(pipe, output))
>> + continue;
>> + /*
>> + * If previous subtest of connector failed, pipe
>> + * attached to that connector is not released.
>> + * Because of that we have to choose the non
>> + * attached pipe for this subtest.
>> + */
>> + if (!igt_pipe_is_free(display, pipe))
>> + continue;
>> +
>> + prepare_test(data, output, pipe);
>> +
>> + /* 10-bit formats are slow, so limit the size. */
>> + afb_id = igt_create_fb(data->fd, 512, 512,
>> DRM_FORMAT_XRGB2101010, 0, &afb);
>> + igt_assert(afb_id);
>> +
>> + draw_hdr_pattern(&afb);
>> +
>> + /* Start in 8bpc. */
>> + igt_plane_set_fb(data->primary, &afb);
>> + /*
>> + * Limiting plane size aswell, since most of gen7 and all of
>> + * gen8 doesn't support plane scaling at all.
>> + */
>> + igt_plane_set_size(data->primary, 512, 512);
>> + igt_output_set_prop_value(data->output,
>> IGT_CONNECTOR_MAX_BPC, 8);
>> + igt_display_commit_atomic(display,
>> DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
>> + /*
>> + * i915 driver doesn't expose max bpc as debugfs entry,
>> + * so limiting assert only for amd driver.
>> + */
>> + if (is_amdgpu_device(data->fd))
>> + assert_output_bpc(data, 8);
>> +
>> + /* Switch to 10bpc. */
>> + igt_output_set_prop_value(data->output,
>> IGT_CONNECTOR_MAX_BPC, 10);
>> + igt_display_commit_atomic(display,
>> DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
>> + if (is_amdgpu_device(data->fd))
>> + assert_output_bpc(data, 10);
>> +
>> + /* Verify that the CRC are equal after DPMS or suspend. */
>> + igt_pipe_crc_collect_crc(data->pipe_crc, &ref_crc);
>> + test_cycle_flags(data, flags);
>> + igt_pipe_crc_collect_crc(data->pipe_crc, &new_crc);
>> +
>> + /* Drop back to 8bpc. */
>> + igt_output_set_prop_value(data->output,
>> IGT_CONNECTOR_MAX_BPC, 8);
>> + igt_display_commit_atomic(display,
>> DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
>> + if (is_amdgpu_device(data->fd))
>> + assert_output_bpc(data, 8);
>> +
>> + /* CRC capture is clamped to 8bpc, so capture should match. */
>> + igt_assert_crc_equal(&ref_crc, &new_crc);
>> +
>> + test_fini(data);
>> + igt_remove_fb(data->fd, &afb);
>> +
>> + /*
>> + * Testing a output with a pipe is enough for HDR
>> + * testing. No ROI in testing the connector with other
>> + * pipes. So break the loop on pipe.
>> + */
>> + break;
>> + }
>> +}
>> +
>> +/* Returns true if an output supports max bpc property. */
>> +static bool has_max_bpc(igt_output_t *output)
>> +{
>> + return igt_output_has_prop(output, IGT_CONNECTOR_MAX_BPC) &&
>> + igt_output_get_prop(output, IGT_CONNECTOR_MAX_BPC);
>> +}
>> +
>> +static void test_bpc_switch(data_t *data, uint32_t flags)
>> +{
>> + igt_output_t *output;
>> + int valid_tests = 0;
>> +
>> + for_each_connected_output(&data->display, output) {
>> + if (!has_max_bpc(output))
>> + continue;
>> +
>> + igt_info("BPC switch test execution on %s\n", output->name);
>> + test_bpc_switch_on_output(data, output, flags);
>> + valid_tests++;
>> + }
>> +
>> + igt_require_f(valid_tests, "No connector found with MAX BPC
>> connector property\n");
>> +}
>> +
>> +igt_main
>> +{
>> + data_t data = { 0 };
>> +
>> + igt_fixture {
>> + data.fd = drm_open_driver_master(DRIVER_AMDGPU | DRIVER_INTEL);
>> +
>> + kmstest_set_vt_graphics_mode();
>> +
>> + igt_display_require(&data.display, data.fd);
>> + igt_require(data.display.is_atomic);
>> +
>> + igt_display_require_output(&data.display);
>> + }
>> +
>> + igt_describe("Tests switching between different display output
>> bpc modes");
>> + igt_subtest("bpc-switch") test_bpc_switch(&data, TEST_NONE);
>> + igt_describe("Tests bpc switch with dpms");
>> + igt_subtest("bpc-switch-dpms") test_bpc_switch(&data, TEST_DPMS);
>> + igt_describe("Tests bpc switch with suspend");
>> + igt_subtest("bpc-switch-suspend") test_bpc_switch(&data,
>> TEST_SUSPEND);
>> +
>> + igt_fixture {
>> + igt_display_fini(&data.display);
>> + }
>> +}
>> diff --git a/tests/meson.build b/tests/meson.build
>> index bc8652aa..6ac2c830 100644
>> --- a/tests/meson.build
>> +++ b/tests/meson.build
>> @@ -38,6 +38,7 @@ test_progs = [
>> 'kms_frontbuffer_tracking',
>> 'kms_getfb',
>> 'kms_hdmi_inject',
>> + 'kms_hdr',
>> 'kms_invalid_dotclock',
>> 'kms_lease',
>> 'kms_legacy_colorkey',
>>
>
More information about the igt-dev
mailing list