[igt-dev] [PATCH i-g-t v5] test/kms_dp_dsc: Basic KMS test to validate VESA DSC on DP/eDP
Srivatsa, Anusha
anusha.srivatsa at intel.com
Wed Dec 12 18:44:23 UTC 2018
>-----Original Message-----
>From: Latvala, Petri
>Sent: Friday, December 7, 2018 2:33 AM
>To: Navare, Manasi D <manasi.d.navare at intel.com>
>Cc: igt-dev at lists.freedesktop.org; Argenziano, Antonio
><antonio.argenziano at intel.com>; Pandiyan, Dhinakaran
><dhinakaran.pandiyan at intel.com>; Ville Syrjälä <ville.syrjala at linux.intel.com>;
>Srivatsa, Anusha <anusha.srivatsa at intel.com>
>Subject: Re: [PATCH i-g-t v5] test/kms_dp_dsc: Basic KMS test to validate VESA
>DSC on DP/eDP
>
>On Wed, Dec 05, 2018 at 12:15:23PM -0800, Manasi Navare wrote:
>> This patch adds a basic kms test to validate the display stream
>> compression functionality if supported on DP/eDP connector.
>> Currently this has only two subtests to force the DSC on all the eDP
>> and DP connectors that support it with default parameters.
>> This will be expanded to add more subtests to tweak DSC parameters.
>>
>> v5:
>> * Fix test cleanup to avoid crash (Petri)
>> v4:
>> * Future proof for more test types (Petri)
>> * Fix alphabetical order (Petri)
>> * s/igt_display_init/igt_display_require (Petri)
>> * Remove blank lines after return (Petri)
>> v3:
>> * Use array of connectors and loop through (Petri)
>> * Also check for FEC on DP connectors (Manasi)
>> * Add a Pipe_A restriction on DP (Ville)
>> v2:
>> * Use IGT wrappers for all (DK, Antonio)
>> * Split into two subtests for eDP and DP types (Petri)
>>
>> Cc: Petri Latvala <petri.latvala at intel.com>
>> Cc: Antonio Argenziano <antonio.argenziano at intel.com>
>> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
>> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
>> Cc: Anusha Srivatsa <anusha.srivatsa at intel.com>
>> Signed-off-by: Manasi Navare <manasi.d.navare at intel.com>
>> ---
>> tests/Makefile.sources | 1 +
>> tests/kms_dp_dsc.c | 258 +++++++++++++++++++++++++++++++++++++++++
>> tests/meson.build | 1 +
>> 3 files changed, 260 insertions(+)
>> create mode 100644 tests/kms_dp_dsc.c
>>
>> diff --git a/tests/Makefile.sources b/tests/Makefile.sources index
>> eedde1e8..a3c24c99 100644
>> --- a/tests/Makefile.sources
>> +++ b/tests/Makefile.sources
>> @@ -54,6 +54,7 @@ TESTS_progs = \
>> kms_crtc_background_color \
>> kms_cursor_crc \
>> kms_cursor_legacy \
>> + kms_dp_dsc \
>> kms_draw_crc \
>> kms_fbcon_fbt \
>> kms_fence_pin_leak \
>> diff --git a/tests/kms_dp_dsc.c b/tests/kms_dp_dsc.c new file mode
>> 100644 index 00000000..e0faec1f
>> --- /dev/null
>> +++ b/tests/kms_dp_dsc.c
>> @@ -0,0 +1,258 @@
>> +/*
>> + * Copyright © 2018 Intel Corporation
>> + *
>> + * 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 (including
>> +the next
>> + * paragraph) 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 AUTHORS OR COPYRIGHT HOLDERS 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.
>> + *
>> + * Displayport Display Stream Compression test
>> + * Until the CRC support is added this needs to be invoked with
>> +--interactive
>> + * to manually verify if the test pattern is seen without corruption
>> +for each
>> + * subtest.
>> + *
>> + * Authors:
>> + * Manasi Navare <manasi.d.navare at intel.com>
>> + *
>> + */
>> +#include "igt.h"
>> +#include "igt_sysfs.h"
>> +#include <errno.h>
>> +#include <getopt.h>
>> +#include <math.h>
>> +#include <stdint.h>
>> +#include <stdbool.h>
>> +#include <strings.h>
>> +#include <unistd.h>
>> +#include <stdlib.h>
>> +#include <signal.h>
>> +#include <time.h>
>> +#include <fcntl.h>
>> +#include <termios.h>
>> +
>> +enum dsc_test_type
>> +{
>> + test_basic_dsc_enable
>> +};
>> +
>> +typedef struct {
>> + int drm_fd;
>> + int debugfs_fd;
>> + uint32_t id;
>> + igt_display_t display;
>> + struct igt_fb fb_test_pattern;
>> + igt_output_t *output;
>> + int mode_valid;
>> + drmModeModeInfo *mode;
>> + drmModeConnector *connector;
>> + drmModeEncoder *encoder;
>> + int crtc;
>> + enum pipe pipe;
>> + char conn_name[128];
>> +} data_t;
>> +
>> +static inline void manual(const char *expected) {
>> + igt_debug_manual_check("all", expected); }
>> +
>> +static bool is_dp_dsc_supported(data_t *data) {
>> + char file_name[128] = {0};
>> + char buf[512];
>> +
>> + strcpy(file_name, data->conn_name);
>> + strcat(file_name, "/i915_dsc_fec_support");
>> + igt_require(igt_debugfs_simple_read(data->debugfs_fd, file_name, buf,
>> + sizeof(buf)) > 0);
>> + igt_debugfs_read(data->drm_fd, file_name, buf);
>> +
>> + return strstr(buf, "DSC_Sink_Support: yes"); }
>> +
>> +static bool is_dp_fec_supported(data_t *data) {
>> + char file_name[128] = {0};
>> + char buf[512];
>> +
>> + strcpy(file_name, data->conn_name);
>> + strcat(file_name, "/i915_dsc_fec_support");
>> + igt_debugfs_read(data->drm_fd, file_name, buf);
>> +
>> + return strstr(buf, "FEC_Sink_Support: yes"); }
>> +
>> +static bool is_dp_dsc_enabled(data_t *data) {
>> + char file_name[128] = {0};
>> + char buf[512];
>> +
>> + strcpy(file_name, data->conn_name);
>> + strcat(file_name, "/i915_dsc_fec_support");
>> + igt_debugfs_read(data->drm_fd, file_name, buf);
>> +
>> + return strstr(buf, "DSC_Enabled: yes"); }
>> +
>> +static void force_dp_dsc_enable(data_t *data) {
>> + char file_name[128] = {0};
>> + int ret;
>> +
>> + strcpy(file_name, data->conn_name);
>> + strcat(file_name, "/i915_dsc_fec_support");
>> + igt_debug ("Forcing DSC enable on %s\n", data->conn_name);
>> + ret = igt_sysfs_write(data->debugfs_fd, file_name, "1", 1);
>> + igt_assert_f(ret > 0, "debugfs_write failed"); }
>> +
>> +static void clear_dp_dsc_enable(data_t *data) {
>> + char file_name[128] = {0};
>> + int ret;
>> +
>> + strcpy(file_name, data->conn_name);
>> + strcat(file_name, "/i915_dsc_fec_support");
>> + igt_debug ("Clearing DSC enable on %s\n", data->conn_name);
>> + ret = igt_sysfs_write(data->debugfs_fd, file_name, "0", 1);
>> + igt_assert_f(ret > 0, "debugfs_write failed"); }
>> +
>> +static void test_cleanup(data_t *data) {
>> + igt_plane_t *primary;
>> +
>> + primary = igt_output_get_plane_type(data->output,
>> + DRM_PLANE_TYPE_PRIMARY);
>> + igt_plane_set_fb(primary, NULL);
>> + igt_display_commit(&data->display);
>> +
>> + igt_remove_fb(data->drm_fd, &data->fb_test_pattern); }
>> +
>> +
>> +/*
>> + * Re-probe connectors and do a modeset with DSC
>> + *
>> + */
>> +static void update_display(data_t *data, enum dsc_test_type
>> +test_type) {
>> + igt_plane_t *primary;
>> + data->mode = igt_output_get_mode(data->output);
>> + data->connector = data->output->config.connector;
>> +
>> + if (data->connector->connector_type ==
>DRM_MODE_CONNECTOR_DisplayPort &&
>> + data->pipe == PIPE_A) {
>> + igt_debug ("DSC not supported on Pipe A on external DP\n");
>> + return;
>> + }
>> +
>> + if (test_type == test_basic_dsc_enable) {
>> + igt_debug ("DSC is supported on %s\n",
>> + data->conn_name);
>> + force_dp_dsc_enable(data);
Petri,
I will do a igt_remove_fb() to avoid leaks.
>> + igt_create_pattern_fb(data->drm_fd, data->mode->hdisplay,
>> + data->mode->vdisplay,
>> + DRM_FORMAT_XRGB8888,
>> + LOCAL_DRM_FORMAT_MOD_NONE,
>> + &data->fb_test_pattern);
>
>This function is called from a loop and data->fb_test_pattern can contain an fb
>already, no? Does this leak?
>
>
>--
>Petri Latvala
>
>
>> + primary = igt_output_get_plane_type(data->output,
>> +
>DRM_PLANE_TYPE_PRIMARY);
>> + igt_plane_set_fb(primary, &data->fb_test_pattern);
>> + igt_display_commit(&data->display);
>> + /* Until we have CRC check support, manually check if RGB test
>pattern has no corruption */
>> + manual ("RGB test pattern without corruption");
>> + igt_assert_f(is_dp_dsc_enabled(data),
>> + "Default DSC enable failed on Connector: %s Pipe:
>%s",
>> + data->conn_name,
>> + kmstest_pipe_name(data->pipe));
>> + clear_dp_dsc_enable(data);
>> + } else {
>> + igt_assert(!"Unknown test type\n");
>> + }
>> +}
>> +
>> +static void run_test(data_t *data, igt_output_t *output,
>> + enum dsc_test_type test_type)
>> +{
>> + enum pipe pipe;
>> +
And here, like you suggested on our conversation, change for_each_pipe_with_valid_output() to for_each_pipe() {
And igt_pipe_connector_valid(pipe, output) - get valid connector for the pipe and set the rest.
Anusha
>> + for_each_pipe_with_valid_output(&data->display, pipe, output) {
>> + igt_output_set_pipe(output, pipe);
>> + data->pipe = pipe;
>> + data->output = output;
>> + update_display(data, test_type);
>> + }
>> +}
>> +
>> +igt_main
>> +{
>> + data_t data = {};
>> + igt_output_t *output;
>> + drmModeRes *res;
>> + drmModeConnector *connector;
>> + int i, test_conn_cnt, test_cnt;
>> + int tests[] = {DRM_MODE_CONNECTOR_eDP,
>DRM_MODE_CONNECTOR_DisplayPort};
>> +
>> + igt_fixture {
>> + data.drm_fd = drm_open_driver_master(DRIVER_ANY);
>> + data.debugfs_fd = igt_debugfs_dir(data.drm_fd);
>> + kmstest_set_vt_graphics_mode();
>> + igt_display_require(&data.display, data.drm_fd);
>> + igt_require(res = drmModeGetResources(data.drm_fd));
>> + }
>> +
>> + for (test_cnt = 0; test_cnt < ARRAY_SIZE(tests); test_cnt++) {
>> +
>> + igt_subtest_f("basic-dsc-enable-%s",
>> + kmstest_connector_type_str(tests[test_cnt])) {
>> + test_conn_cnt = 0;
>> + for (i = 0; i < res->count_connectors; i++) {
>> + connector =
>drmModeGetConnectorCurrent(data.drm_fd,
>> + res-
>>connectors[i]);
>> + if (connector->connection !=
>DRM_MODE_CONNECTED ||
>> + connector->connector_type !=
>> + tests[test_cnt])
>> + continue;
>> + test_conn_cnt++;
>> + output =
>igt_output_from_connector(&data.display, connector);
>> + sprintf(data.conn_name, "%s-%d",
>> + kmstest_connector_type_str(connector-
>>connector_type),
>> + test_conn_cnt);
>> + igt_require_f(is_dp_dsc_supported(&data),
>> + "DSC not supported on connector %s
>\n",
>> + data.conn_name);
>> + if (connector->connector_type ==
>DRM_MODE_CONNECTOR_DisplayPort)
>> +
> igt_require_f(is_dp_fec_supported(&data),
>> + "DSC cannot be enabled
>without FEC on %s\n",
>> + data.conn_name);
>> + run_test(&data, output, test_basic_dsc_enable);
>> + }
>> + if (data.output)
>> + test_cleanup(&data);
>> + igt_skip_on(test_conn_cnt == 0);
>> + }
>> + }
>> +
>> + igt_fixture {
>> + drmModeFreeConnector(connector);
>> + drmModeFreeResources(res);
>> + close(data.debugfs_fd);
>> + close(data.drm_fd);
>> + igt_display_fini(&data.display);
>> + }
>> + igt_exit();
>> +}
>> diff --git a/tests/meson.build b/tests/meson.build
>> index b8a6e61b..e14ab2b4 100644
>> --- a/tests/meson.build
>> +++ b/tests/meson.build
>> @@ -25,6 +25,7 @@ test_progs = [
>> 'kms_crtc_background_color',
>> 'kms_cursor_crc',
>> 'kms_cursor_legacy',
>> + 'kms_dp_dsc',
>> 'kms_draw_crc',
>> 'kms_fbcon_fbt',
>> 'kms_fence_pin_leak',
>> --
>> 2.19.1
>>
More information about the igt-dev
mailing list