[igt-dev] [PATCH i-g-t v1 2/6] [intel-gfx] tests/pm_dc : Added new test to verify Display C States. Currently this test validate DC5 upon PSR entry for Icelake, Skylake and Broxton platform.
Dhinakaran Pandiyan
dhinakaran.pandiyan at intel.com
Thu Oct 4 00:13:48 UTC 2018
On Tuesday, October 2, 2018 6:25:13 AM PDT Imre Deak wrote:
> On Wed, Sep 26, 2018 at 11:59:19AM -0400, Jyoti Yadav wrote:
> > Signed-off-by: Jyoti Yadav <jyoti.r.yadav at intel.com>
> > ---
> >
> > tests/pm_dc.c | 180
> > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file
> > changed, 180 insertions(+)
> > create mode 100644 tests/pm_dc.c
> >
> > diff --git a/tests/pm_dc.c b/tests/pm_dc.c
> > new file mode 100644
> > index 0000000..db38ccf
> > --- /dev/null
> > +++ b/tests/pm_dc.c
> > @@ -0,0 +1,180 @@
> > +/*
> > + * Copyright © 2013 Intel Corporation
This has to be updated?
> > + *
> > + * 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;
> > +} data_t;
> > +
> > +bool has_runtime_pm;
> > +
> > +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)
> > +{
> > + 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 read_dc3_dc5_counter(uint32_t dev_id)
> > +{
> > + uint32_t dc3_dc5_count;
> > +
> > + if (IS_SKYLAKE(dev_id) || IS_ICELAKE(dev_id))
> > + dc3_dc5_count = INREG(SKL_CSR_DC3_DC5_COUNT);
> > + else if (IS_BROXTON(dev_id))
> > + dc3_dc5_count = INREG(BXT_CSR_DC3_DC5_COUNT);
>
> The counters are available already in the DMC debugfs file, so no need
> to duplicate the readout and platform check for that.
>
> > + else {
> > + igt_warn("Currently DC5 support is for ICL, SKL and BXT Platform
> > (%s)\n", + __func__);
> > + return -EPERM;
>
> We want to skip the test if the DC counters don't exist, so we'd need an
> igt_require() instead of the error return.
>
> > + }
> > + return dc3_dc5_count;
> > +}
> > +
> > +static void test_dc5(data_t *data)
> > +{
> > + uint32_t dc3_dc5_counter_before_psr, dc3_dc5_counter_after_psr;
>
> You could use shorter names.
>
> > +
> > + dc3_dc5_counter_before_psr = read_dc3_dc5_counter(data->devid);
> > + setup_primary(data);
> > + igt_assert(psr_wait_entry(data->debugfs_fd));
> > + dc3_dc5_counter_after_psr = read_dc3_dc5_counter(data->devid);
>
> I wonder if this would always work or if we'd need to wait for the
> counter to increase with a timeout.
PSR needs some time to become active (depends on the idle frame count) and DMC
takes over after that, so you'll need to wait after entry. Around a second
should be good, I think.
>
> > + igt_require_f(dc3_dc5_counter_after_psr > dc3_dc5_counter_before_psr,
> > + "DC5 State is not achieved\n");
This should be a failure.
igt_assert_f(after > before, " before %d after %d"...)
> > +}
> > +
> > +int main(int argc, char *argv[])
> > +{
> > + data_t data = {};
> > +
> > + igt_skip_on_simulation();
> > + 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);
>
> psr_enable() should be called from the PSR specific subtest.
>
> > + has_runtime_pm = igt_setup_runtime_pm();
> > + igt_info("Runtime PM support: %d\n", has_runtime_pm);
> > + igt_require(has_runtime_pm);
> > + igt_require(dmc_loaded(data.debugfs_fd));
> > +
> > + igt_require_f(sink_support(&data),
> > + "Sink does not support PSR\n");
> > + display_init(&data);
> > + }
> > +
> > + igt_subtest("DC5_state") {
> > + test_dc5(&data);
> > + cleanup(&data);
> > + }
> > + igt_fixture {
> > + psr_disable(data.debugfs_fd);
>
> We want to _restore_ the PSR state not disable it, and that's done
> already from an exit handler.
>
> > + close(data.debugfs_fd);
> > + display_fini(&data);
> > + }
> > +
> > + igt_exit();
> > +}
>
> _______________________________________________
> igt-dev mailing list
> igt-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
More information about the igt-dev
mailing list