[igt-dev] [PATCH i-g-t 2/8] tests/i915/i915_pm_dc: Added new test to verify Display C States
Arkadiusz Hiler
arkadiusz.hiler at intel.com
Wed Sep 18 13:11:17 UTC 2019
On Tue, Sep 17, 2019 at 02:40:46PM +0300, Petri Latvala wrote:
> On Thu, Sep 12, 2019 at 01:46:32PM +0530, Jeevan B wrote:
> > From: Jyoti Yadav <jyoti.r.yadav at intel.com>
> >
> > Currently this test validates DC5 upon PSR entry for supported platforms.
> > Added new file for compilation inside Makefile and Meson.
> >
> > v2: Used the debugfs entry for DC counters instead of Registers.
> > Used shorter names for variables.
> > Introduced timeout to read DC counters.
> > v3: one second timeout is introduced to read DC counters.
> > Skip the subtest if counters are not available for that platform.
> > v4: Rebased, to fix compilation error in psr_enable(), psr_wait_entry()
> > and replaced igt_display_init() by igt_display_require() in pm_rpm.c.
> > Addressed the review comment by removing unused function display_init,
> > removing redundant igt commit suggested by Imre.
> > Fixed typo in read_dc_counter().
> > v5: Addressed the review comment by removing redundant read_dc_counter(),
> > clubbed cleanup() function in test_dc_state_psr() suggested by Imre.
> > Rearranged preprocessor directives at one place.
> > Fixed compilation warning by adding function prototype for
> > dc_state_wait_entry() and check_dc_counter().
> > Listing actual change in patch set changelog to make review easier.
> > v6: Fixed "i915_edp_psr_status" string to "Sink support: yes".
> > v7: Rebased since test name changed from "pm_dc" to "i915_pm_dc",
> > this will align with other PM tests.
> > also changed the DC5/6 counter check timeout to 3 second.
> > v8: On few platform SATA i/p is blocking deeper package C states.
> > used igt_pm_enable_sata_link_power_management() function in order
> > to use min_power policy.
> > v9: Removed redundant igt_pm_remove_sata_link_power_management()
> > as sata link policy values will be restored by exit handler.
> > v10: changed cleanup() to cleanup_dc_psr() symmetric to cleanup_dc_dpms().
> > v11: Removed unwanted headers. [Imre]
> >
> > Signed-off-by: Jyoti Yadav <jyoti.r.yadav at intel.com>
> > Signed-off-by: Anshuman Gupta <anshuman.gupta at intel.com>
> > ---
> > tests/Makefile.sources | 3 +
> > tests/i915/i915_pm_dc.c | 211 ++++++++++++++++++++++++++++++++++++++++++++++++
> > tests/meson.build | 1 +
> > 3 files changed, 215 insertions(+)
> > create mode 100644 tests/i915/i915_pm_dc.c
> >
> > diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> > index 549a350..06a0a48 100644
> > --- a/tests/Makefile.sources
> > +++ b/tests/Makefile.sources
> > @@ -520,6 +520,9 @@ i915_pm_rc6_residency_SOURCES = i915/i915_pm_rc6_residency.c
> > TESTS_progs += i915_pm_rpm
> > i915_pm_rpm_SOURCES = i915/i915_pm_rpm.c
> >
> > +TESTS_progs += i915_pm_dc
> > +i915_pm_dc_SOURCES = i915/i915_pm_dc.c
> > +
> > TESTS_progs += i915_pm_rps
> > i915_pm_rps_SOURCES = i915/i915_pm_rps.c
> >
> > diff --git a/tests/i915/i915_pm_dc.c b/tests/i915/i915_pm_dc.c
> > new file mode 100644
> > index 0000000..c0e41c3
> > --- /dev/null
> > +++ b/tests/i915/i915_pm_dc.c
> > @@ -0,0 +1,211 @@
> > +/*
> > + * 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 <errno.h>
> > +#include <stdbool.h>
> > +#include <stdio.h>
> > +#include <string.h>
> > +#include "igt.h"
> > +#include "igt_psr.h"
> > +#include "igt_sysfs.h"
> > +#include "limits.h"
> > +
> > +/* DC State Flags */
> > +#define CHECK_DC5 1
> > +#define CHECK_DC6 2
> > +
> > +typedef struct {
> > + int drm_fd;
> > + int debugfs_fd;
> > + uint32_t devid;
> > + igt_display_t display;
> > + struct igt_fb fb_white;
> > + enum psr_mode op_psr_mode;
> > + drmModeModeInfo *mode;
> > + igt_output_t *output;
> > +} data_t;
> > +
> > +static bool dc_state_wait_entry(int drm_fd, int dc_flag, int prev_dc_count);
> > +static void check_dc_counter(int drm_fd, int dc_flag, uint32_t prev_dc_count);
> > +
> > +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_fini(data_t *data)
> > +{
> > + igt_display_fini(&data->display);
> > +}
> > +
> > +static bool edp_psr_sink_support(data_t *data)
> > +{
> > + char buf[512];
> > +
> > + igt_debugfs_simple_read(data->debugfs_fd, "i915_edp_psr_status",
> > + buf, sizeof(buf));
> > +
> > + return strstr(buf, "Sink support: yes");
> > +}
> > +
> > +static void cleanup_dc_psr(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_plane_set_fb(primary, &data->fb_white);
> > + igt_display_commit(&data->display);
> > +}
> > +
> > +static uint32_t get_dc_counter(char *dc_data)
> > +{
> > + char *e;
> > + long ret;
> > + char *s = strchr(dc_data, ':');
> > +
> > + assert(s);
> > + s++;
> > + ret = strtol(s, &e, 10);
> > + assert(((ret != LONG_MIN && ret != LONG_MAX) || errno != ERANGE) &&
> > + e > s && *e == '\n' && ret >= 0);
> > + return ret;
> > +}
> > +
> > +static uint32_t read_dc_counter(uint32_t drm_fd, int dc_flag)
> > +{
> > + char buf[4096];
> > + char *str;
> > +
> > + igt_debugfs_read(drm_fd, "i915_dmc_info", buf);
> > +
> > + if (dc_flag & CHECK_DC5)
> > + str = strstr(buf, "DC3 -> DC5 count");
> > + else if (dc_flag & CHECK_DC6)
> > + str = strstr(buf, "DC5 -> DC6 count");
> > +
> > + /* Check DC5/DC6 counter is available for the platform.
> > + * Skip the test if counter is not available.
> > + */
> > + igt_skip_on_f(!str, "DC%d counter is not available\n",
> > + dc_flag & CHECK_DC5 ? 5 : 6);
> > + return get_dc_counter(str);
> > +}
> > +
> > +static bool dc_state_wait_entry(int drm_fd, int dc_flag, int prev_dc_count)
> > +{
> > + return igt_wait(read_dc_counter(drm_fd, dc_flag) >
> > + prev_dc_count, 3000, 100);
> > +}
> > +
> > +static void check_dc_counter(int drm_fd, int dc_flag, uint32_t prev_dc_count)
> > +{
> > + igt_assert_f(dc_state_wait_entry(drm_fd, dc_flag, prev_dc_count),
> > + "DC%d state is not achieved\n",
> > + dc_flag & CHECK_DC5 ? 5 : 6);
> > +}
> > +
> > +static void test_dc_state_psr(data_t *data, int dc_flag)
> > +{
> > + uint32_t dc_counter_before_psr;
> > +
> > + dc_counter_before_psr = read_dc_counter(data->drm_fd, dc_flag);
> > + setup_output(data);
> > + setup_primary(data);
> > + igt_assert(psr_wait_entry(data->debugfs_fd, data->op_psr_mode));
> > + check_dc_counter(data->drm_fd, dc_flag, dc_counter_before_psr);
> > + cleanup_dc_psr(data);
> > +}
> > +
> > +IGT_TEST_DESCRIPTION("Display Power DC states IGT Test");
> > +int main(int argc, char *argv[])
> > +{
> > + bool has_runtime_pm;
> > + 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);
> > + igt_pm_enable_sata_link_power_management();
> > + 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_display_require(&data.display, data.drm_fd);
> > + }
> > +
> > + igt_describe("DC5 igt test with PSR Sleep state");
> > + igt_subtest("dc5-psr") {
>
>
> Can the documentation be fleshed out a bit more? For one, it's
> redundant for an IGT test to state that it's an IGT test (applies to
> both the igt_describe calls and the IGT_TEST_DESCRIPTION string).
>
> "DC5 test with PSR Sleep state" is not clear what the test is
> testing. Something like "Test that DC5 state is reached in PSR sleep
> state"?
I would suggest changing them to:
IGT_TEST_DESCRIPTION("Verify that we can enter display's DC states under various conditions.");
igt_describe("Make sure that we enter DC5 with PSR enabled.");
Feel free to suggest something else and correct me if I got anything
wrong. I have just skimmed through the tests.
You need to make similar changes to all the other test introduced in
this patch-series.
--
Cheers,
Arek
More information about the igt-dev
mailing list