[igt-dev] [PATCH 4/7] tests/kms_hdr: Add bpc switch subtests
Sharma, Swati2
swati2.sharma at intel.com
Mon Feb 3 08:49:14 UTC 2020
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',
>
--
~Swati Sharma
More information about the igt-dev
mailing list