[igt-dev] [PATCH i-g-t v2 2/5] [intel-gfx] tests/pm_dc : Added new test to verify Display C States.
Imre Deak
imre.deak at intel.com
Mon Oct 15 15:18:15 UTC 2018
Extra space after ':' in the subject line.
On Wed, Oct 10, 2018 at 08:01:45AM -0400, Jyoti Yadav wrote:
> Currently this test validate DC5 upon PSR entry for supported platforms.
> Added new file for compilation inside Makefile and Meson.
>
> Signed-off-by: Jyoti Yadav <jyoti.r.yadav at intel.com>
> ---
> tests/Makefile.sources | 1 +
> tests/meson.build | 1 +
> tests/pm_dc.c | 217 +++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 219 insertions(+)
> create mode 100644 tests/pm_dc.c
>
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index c84933f..d491420 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -214,6 +214,7 @@ TESTS_progs = \
> pm_lpsp \
> pm_rc6_residency \
> pm_rpm \
> + pm_dc \
> pm_rps \
> pm_sseu \
> prime_busy \
> diff --git a/tests/meson.build b/tests/meson.build
> index 17deb94..50b0687 100644
> --- a/tests/meson.build
> +++ b/tests/meson.build
> @@ -188,6 +188,7 @@ test_progs = [
> 'pm_lpsp',
> 'pm_rc6_residency',
> 'pm_rpm',
> + 'pm_dc',
> 'pm_rps',
> 'pm_sseu',
> 'prime_busy',
> diff --git a/tests/pm_dc.c b/tests/pm_dc.c
> new file mode 100644
> index 0000000..4125d83
> --- /dev/null
> +++ b/tests/pm_dc.c
> @@ -0,0 +1,217 @@
> +/*
> + * 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.
> + *
> + */
> +
> +#include "igt.h"
> +#include "igt_sysfs.h"
> +#include "igt_psr.h"
> +#include <errno.h>
> +#include <stdbool.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include "intel_bufmgr.h"
> +#include "intel_io.h"
> +
> +
> +typedef struct {
> + int drm_fd;
> + int debugfs_fd;
> + uint32_t devid;
> + igt_display_t display;
> + struct igt_fb fb_white;
> + drmModeModeInfo *mode;
> + igt_output_t *output;
> + int dc_flags;
> +} data_t;
> +
> +bool has_runtime_pm;
Can't see the above used outside of main().
> +/* DC State Flags */
> +#define CHECK_DC5 1
> +#define CHECK_DC6 2
> +
> +static void setup_output(data_t *data)
> +{
> + igt_display_t *display = &data->display;
> + igt_output_t *output;
> + enum pipe pipe;
> +
> + for_each_pipe_with_valid_output(display, pipe, output) {
> + drmModeConnectorPtr c = output->config.connector;
> +
> + if (c->connector_type != DRM_MODE_CONNECTOR_eDP)
> + continue;
> +
> + igt_output_set_pipe(output, pipe);
> + data->output = output;
> + data->mode = igt_output_get_mode(output);
> +
> + return;
> + }
> +}
> +
> +static void display_init(data_t *data)
> +{
> + igt_display_init(&data->display, data->drm_fd);
> + setup_output(data);
> +}
> +
> +static void display_fini(data_t *data)
> +{
> + igt_display_fini(&data->display);
> +}
> +
> +static bool sink_support(data_t *data)
The function name should reflect what it really checks.
> +{
> + char buf[512];
> +
> + igt_debugfs_simple_read(data->debugfs_fd, "i915_edp_psr_status",
> + buf, sizeof(buf));
> +
> + return strstr(buf, "Sink_Support: yes\n");
> +}
> +
> +static void 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_white);
> +}
> +
> +static void setup_primary(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_create_color_fb(data->drm_fd,
> + data->mode->hdisplay, data->mode->vdisplay,
> + DRM_FORMAT_XRGB8888,
> + LOCAL_I915_FORMAT_MOD_X_TILED,
> + 1.0, 1.0, 1.0,
> + &data->fb_white);
> + igt_display_commit(&data->display);
> +
> + igt_plane_set_fb(primary, &data->fb_white);
> + igt_display_commit(&data->display);
> +}
> +
> +static uint32_t get_dc_counter(char *dc_data)
> +{
> + /* store the DC string till new line inside array */
> + char buf[20];
> + int i = 0;
> + char *token;
> + unsigned int counter;
> +
> + while (dc_data[i] != '\n') {
> + buf[i] = dc_data[i];
> + i++;
> + }
> + buf[i] = '\0';
> + token = strtok(buf, ":");
> + token = strtok(NULL, ":");
> + counter = strtoul(token, NULL, 10);
> + return counter;
> +}
The following would be safer:
char *s = strchr(dc_data, ':');
char *e;
long ret;
assert(s);
s++;
ret = strtol(s, &e, 10);
assert(((ret != LONG_MIN && ret != LONG_MAX) || errno != ERANGE) &&
(ret != 0 || errno =) &&
e > s && *e == '\n' &&
ret >= 0);
return ret;
> +
> +static uint32_t read_dc_counter(uint32_t drm_fd, int dc_flags)
> +{
> + char buf[4096];
> + char *str;
> +
> + igt_debugfs_read(drm_fd, "i915_dmc_info", buf);
> + igt_skip_on(buf[0] == '\0');
This can't happen since we skip the test if dmc_loaded() returns false.
> + if (dc_flags & CHECK_DC5)
> + str = strstr(buf, "DC3 -> DC5 count");
> + else if (dc_flags & CHECK_DC6)
> + str = strstr(buf, "DC5 -> DC6 count");
> + igt_assert(str);
> + return get_dc_counter(str);
> +}
> +
> +bool dc_state_wait_entry(data_t *data, int prev_dc_count)
> +{
> + return igt_wait(read_dc_counter(data->drm_fd, data->dc_flags) > prev_dc_count, 5000, 100);
I think 5 second would already point to some problem, the 1 second
suggested by DK seems ok to me.
> +}
> +
> +void check_dc_counter(data_t *data, uint32_t prev_dc_count)
> +{
> + if (data->dc_flags & CHECK_DC5) {
> + igt_assert_f(dc_state_wait_entry(data, prev_dc_count),
> + "DC5 state is not achieved\n");
> + } else if (data->dc_flags & CHECK_DC6) {
> + igt_assert_f(dc_state_wait_entry(data, prev_dc_count),
> + "DC6 state is not achieved\n");
> + }
igt_assert_f(dc_state_wait_entry(data, prev_dc_count),
"DC%d state is not achieved\n",
data->dc_flags & CHECK_DC5 ? 5 : 6);
would be simpler to read.
> +}
> +
> +static void test_dc_state_psr(data_t *data)
> +{
> + uint32_t dc_counter_before_psr;
> +
> + dc_counter_before_psr = read_dc_counter(data->drm_fd, data->dc_flags);
> + setup_primary(data);
> + igt_assert(psr_wait_entry(data->debugfs_fd));
> + check_dc_counter(data, dc_counter_before_psr);
> +}
> +
> +int main(int argc, char *argv[])
> +{
> + data_t data = {};
> +
> + igt_skip_on_simulation();
> + igt_subtest_init(argc, argv);
> + igt_fixture {
> + data.drm_fd = drm_open_driver_master(DRIVER_INTEL);
> + data.debugfs_fd = igt_debugfs_dir(data.drm_fd);
> + igt_require(data.debugfs_fd != -1);
> + kmstest_set_vt_graphics_mode();
> + data.devid = intel_get_drm_devid(data.drm_fd);
> + psr_enable(data.debugfs_fd);
> + has_runtime_pm = igt_setup_runtime_pm();
> + igt_info("Runtime PM support: %d\n", has_runtime_pm);
> + igt_require(has_runtime_pm);
> + igt_require(igt_pm_dmc_loaded(data.debugfs_fd));
> +
> + igt_require_f(sink_support(&data),
> + "Sink does not support PSR\n");
Would be clearer to put the above and psr_enable() to the relevant
subtests.
> + display_init(&data);
> + }
> +
> + igt_subtest("dc5-psr") {
> + data.dc_flags |= CHECK_DC5;
You could just pass the flag directly to the test function.
> + test_dc_state_psr(&data);
> + cleanup(&data);
> + }
> + igt_fixture {
> + close(data.debugfs_fd);
> + display_fini(&data);
> + }
> +
> + igt_exit();
> +}
> --
> 1.9.1
>
More information about the igt-dev
mailing list