[igt-dev] [PATCH i-g-t v2 7/9] tests/psr: Add the same test coverage that we have for PSR1 to PSR2
Souza, Jose
jose.souza at intel.com
Fri Jan 11 20:56:30 UTC 2019
On Fri, 2019-01-11 at 11:49 -0800, Dhinakaran Pandiyan wrote:
> On Thu, 2019-01-10 at 17:12 -0800, Souza, Jose wrote:
> > On Thu, 2019-01-10 at 15:52 -0800, Dhinakaran Pandiyan wrote:
> > > On Thu, 2019-01-03 at 06:36 -0800, José Roberto de Souza wrote:
> > > > The main tests for PSR1 check if hardware tracking is detecting
> > > > changes in planes when modifing it in different ways and now
> > > > those tests will also run for PSR2 if supported by source and
> > > > sink.
> > > >
> > > > Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
> > > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
> > > > Signed-off-by: José Roberto de Souza <jose.souza at intel.com>
> > > > ---
> > > > tests/kms_psr.c | 112
> > > > +++++++++++++++++++++++++++++++++++++++++++++-
> > > > --
> > > > 1 file changed, 105 insertions(+), 7 deletions(-)
> > > >
> > > > diff --git a/tests/kms_psr.c b/tests/kms_psr.c
> > > > index 855679b0..d4d38320 100644
> > > > --- a/tests/kms_psr.c
> > > > +++ b/tests/kms_psr.c
> > > > @@ -60,6 +60,7 @@ typedef struct {
> > > > int drm_fd;
> > > > int debugfs_fd;
> > > > enum operations op;
> > > > + bool op_psr2;
> > > > uint32_t devid;
> > > > uint32_t crtc_id;
> > > > igt_display_t display;
> > > > @@ -71,6 +72,7 @@ typedef struct {
> > > > drmModeModeInfo *mode;
> > > > igt_output_t *output;
> > > > bool with_psr_disabled;
> > > > + bool supports_psr2;
> > > > } data_t;
> > > >
> > > > static void create_cursor_fb(data_t *data)
> > > > @@ -194,12 +196,22 @@ static bool sink_support(data_t *data)
> > > > return data->with_psr_disabled ||
> > > > psr_sink_support(data-
> > > > > debugfs_fd);
> > > >
> > > > }
> > > >
> > > > -static bool psr_wait_entry_if_enabled(data_t *data)
> > > > +static bool sink_support_psr2(data_t *data)
> > > > {
> > > > - if (data->with_psr_disabled)
> > > > - return true;
> > > > + return data->with_psr_disabled ||
> > > > psr2_sink_support(data-
> > > > > debugfs_fd);
> > > >
> > > > +}
> > > >
> > > > - return psr_wait_entry(data->debugfs_fd);
> > > > +static bool psr_wait_entry_if_enabled(data_t *data)
> > > > +{
> > > > + if (!data->op_psr2) {
> > > > + if (data->with_psr_disabled)
> > > > + return true;
> > > > + return psr_wait_entry(data->debugfs_fd);
> > > > + } else {
> > > > + if (data->with_psr_disabled)
> > > > + return true;
> > > > + return psr2_wait_deep_sleep(data->debugfs_fd);
> > > > + }
> > > > }
> > >
> > > Unless I am missing something, we can simplify all these changes
> > > by
> > > pushing the PSR mode down through the call chain.
> > >
> > >
> > > -static bool psr_active(int debugfs_fd, bool check_active)
> > > +static bool psr_active(int debugfs_fd, enum psr_mode mode, bool
> > > check_active)
> > > {
> >
> > I added the PSR2 functions with other names to avoid any
> > misinterpretation as PSR2 could be active in deep sleep state and
> > doing
> > selective update and in the states between and in the kms_psr2_su
> > we
> > need to check both states to properly test.
>
> Selective update testing does not need modifying the behavior of any
> of
> these functions, does it? Both PSR1 and PSR2(non-SU) tests wait until
> PSR is in an active state, do some operation and then verify we are
> out
> of the state.
Yes but after perform some operation it can keep into a active state in
PSR2 that is why psr_wait_exit() is not a good name. Same for
psr_wait_entry() it is wait entry in deep sleep? or wait entry in PSR
active?
> Since they follow the same test steps, I believe it is
> better to share the interface. We also want kms_fbt_fbcon and
> kms_frontbuffer_tracking to setup PSR2, that is another reason
> psr_wait_entry()/psr_wait_exit() in the library should handle PSR1
> v/s
> PSR2 instead of the callers in kms_psr.
>
>
> >
> > I'm okay in adding the PSR mode to enable(), disable(),
> > sink_support()
> > and reuse the internal functions too but in the other functions the
> > code saved is not worth. What do you think DK?
> >
> > > bool active;
> > > char buf[PSR_STATUS_MAX_LEN];
> > > + char *s = mode == PSR_MODE_2 ? "DEEP_SLEEP" ? "SRDENT";
> > >
> > > igt_debugfs_simple_read(debugfs_fd,
> > > "i915_edp_psr_status",
> > > buf,
> > > sizeof(buf));
> > > active = strstr(buf, "HW Enabled & Active bit: yes\n") &&
> > > - (strstr(buf, "SRDENT") || strstr(buf, "SLEEP"));
> > > + strstr(buf, s);
> > > return check_active ? active : !active;
> > > }
> > >
> > > -bool psr_wait_entry(int debugfs_fd)
> > > +bool psr_wait_entry(int debugfs_fd, int psr_mode)
> > > {
> > > - return igt_wait(psr_active(debugfs_fd, true), 500, 20);
> > > + return igt_wait(psr_active(debugfs_fd, true, psr_mode),
> > > 500,
> > > 20);
> > > }
> > >
> > > -bool psr_wait_exit(int debugfs_fd)
> > > +bool psr_wait_exit(int debugfs_fd, int psr_mode)
> > > {
> > > return igt_wait(psr_active(debugfs_fd, false), 40, 10);
> > > }
> > > diff --git a/tests/kms_psr.c b/tests/kms_psr.c
> > > index 855679b0..bb90b994 100644
> > > --- a/tests/kms_psr.c
> > > +++ b/tests/kms_psr.c
> > > @@ -199,7 +199,7 @@ static bool psr_wait_entry_if_enabled(data_t
> > > *data)
> > > if (data->with_psr_disabled)
> > > return true;
> > >
> > > - return psr_wait_entry(data->debugfs_fd);
> > > + return psr_wait_entry(data->debugfs_fd, data->psr_mode);
> > > }
> > >
> > > static inline void manual(const char *expected)
> > > @@ -289,7 +289,7 @@ static void run_test(data_t *data)
> > > expected = "screen GREEN";
> > > break;
> > > }
> > > - igt_assert(psr_wait_exit(data->debugfs_fd));
> > > + igt_assert(psr_wait_exit(data->debugfs_fd, data-
> > > > psr_mode));
> > > manual(expected);
> > > }
> > >
> > >
> > >
> > > >
> > > > static inline void manual(const char *expected)
> > > > @@ -289,11 +301,16 @@ static void run_test(data_t *data)
> > > > expected = "screen GREEN";
> > > > break;
> > > > }
> > > > - igt_assert(psr_wait_exit(data->debugfs_fd));
> > > > +
> > > > + if (!data->op_psr2)
> > > > + igt_assert(psr_wait_exit(data->debugfs_fd));
> > > > + else
> > > > + igt_assert(psr2_wait_update(data->debugfs_fd));
> > > > manual(expected);
> > > > }
> > > >
> > > > -static void test_cleanup(data_t *data) {
> > > > +static void test_cleanup(data_t *data)
> > > > +{
> > > > igt_plane_t *primary;
> > > >
> > > > primary = igt_output_get_plane_type(data->output,
> > > > @@ -304,6 +321,11 @@ static void test_cleanup(data_t *data) {
> > > >
> > > > igt_remove_fb(data->drm_fd, &data->fb_green);
> > > > igt_remove_fb(data->drm_fd, &data->fb_white);
> > > > +
> > > > + /* switch to PSR1 again */
> > > > + if (data->op_psr2 && !data->with_psr_disabled)
> > > > + psr_enable(data->debugfs_fd);
> > > > + data->op_psr2 = false;
> > > > }
> > > >
> > > > static void setup_test_plane(data_t *data, int test_plane)
> > > > @@ -311,6 +333,11 @@ static void setup_test_plane(data_t *data,
> > > > int
> > > > test_plane)
> > > > uint32_t white_h, white_v;
> > > > igt_plane_t *primary, *sprite, *cursor;
> > > >
> > > > + if (data->op_psr2 && !data->with_psr_disabled) {
> > > > + igt_require(data->supports_psr2);
> > > > + psr2_enable(data->debugfs_fd);
> > > > + }
> > > > +
> > > > igt_create_color_fb(data->drm_fd,
> > > > data->mode->hdisplay, data->mode-
> > > > >vdisplay,
> > > > DRM_FORMAT_XRGB8888,
> > > > @@ -391,7 +418,7 @@ static int opt_handler(int opt, int
> > > > opt_index,
> > > > void *_data)
> > > > int main(int argc, char *argv[])
> > > > {
> > > > const char *help_str =
> > > > - " --no-psr\tRun test without PSR.";
> > > > + " --no-psr\tRun test without PSR/PSR2.\n";
> > > > static struct option long_options[] = {
> > > > {"no-psr", 0, 0, 'n'},
> > > > { 0, 0, 0, 0 }
> > > > @@ -414,6 +441,7 @@ int main(int argc, char *argv[])
> > > >
> > > > igt_require_f(sink_support(&data),
> > > > "Sink does not support PSR\n");
> > > > + data.supports_psr2 = sink_support_psr2(&data);
> > > >
> > > > data.bufmgr =
> > > > drm_intel_bufmgr_gem_init(data.drm_fd,
> > > > 4096);
> > > > igt_assert(data.bufmgr);
> > > > @@ -428,6 +456,13 @@ int main(int argc, char *argv[])
> > > > test_cleanup(&data);
> > > > }
> > > >
> > > > + igt_subtest("psr2_basic") {
> > > > + data.op_psr2 = true;
> > > > + setup_test_plane(&data,
> > > > DRM_PLANE_TYPE_PRIMARY);
> > > > + igt_assert(psr_wait_entry_if_enabled(&data));
> > > > + test_cleanup(&data);
> > > > + }
> > > > +
> > > > igt_subtest("no_drrs") {
> > > > setup_test_plane(&data,
> > > > DRM_PLANE_TYPE_PRIMARY);
> > > > igt_assert(psr_wait_entry_if_enabled(&data));
> > > > @@ -435,6 +470,14 @@ int main(int argc, char *argv[])
> > > > test_cleanup(&data);
> > > > }
> > > >
> > > > + igt_subtest("psr2_no_drrs") {
> > > > + data.op_psr2 = true;
> > > > + setup_test_plane(&data,
> > > > DRM_PLANE_TYPE_PRIMARY);
> > > > + igt_assert(psr_wait_entry_if_enabled(&data));
> > > > + igt_assert(drrs_disabled(&data));
> > > > + test_cleanup(&data);
> > > > + }
> > > > +
> > > > for (op = PAGE_FLIP; op <= RENDER; op++) {
> > > > igt_subtest_f("primary_%s", op_str(op)) {
> > > > data.op = op;
> > > > @@ -445,6 +488,17 @@ int main(int argc, char *argv[])
> > > > }
> > > > }
> > > >
> > > > + for (op = PAGE_FLIP; op <= RENDER; op++) {
> > > > + igt_subtest_f("psr2_primary_%s", op_str(op)) {
> > > > + data.op_psr2 = true;
> > > > + data.op = op;
> > > > + setup_test_plane(&data,
> > > > DRM_PLANE_TYPE_PRIMARY);
> > > > + igt_assert(psr_wait_entry_if_enabled(&d
> > > > ata));
> > > > + run_test(&data);
> > > > + test_cleanup(&data);
> > > > + }
> > > > + }
> > > > +
> > > > for (op = MMAP_GTT; op <= PLANE_ONOFF; op++) {
> > > > igt_subtest_f("sprite_%s", op_str(op)) {
> > > > data.op = op;
> > > > @@ -455,6 +509,17 @@ int main(int argc, char *argv[])
> > > > }
> > > > }
> > > >
> > > > + for (op = MMAP_GTT; op <= PLANE_ONOFF; op++) {
> > > > + igt_subtest_f("psr2_sprite_%s", op_str(op)) {
> > > > + data.op_psr2 = true;
> > > > + data.op = op;
> > > > + setup_test_plane(&data,
> > > > DRM_PLANE_TYPE_OVERLAY);
> > > > + igt_assert(psr_wait_entry_if_enabled(&d
> > > > ata));
> > > > + run_test(&data);
> > > > + test_cleanup(&data);
> > > > + }
> > > > + }
> > > > +
> > > > for (op = MMAP_GTT; op <= PLANE_ONOFF; op++) {
> > > > igt_subtest_f("cursor_%s", op_str(op)) {
> > > > data.op = op;
> > > > @@ -465,6 +530,17 @@ int main(int argc, char *argv[])
> > > > }
> > > > }
> > > >
> > > > + for (op = MMAP_GTT; op <= PLANE_ONOFF; op++) {
> > > > + igt_subtest_f("psr2_cursor_%s", op_str(op)) {
> > > > + data.op_psr2 = true;
> > > > + data.op = op;
> > > > + setup_test_plane(&data,
> > > > DRM_PLANE_TYPE_CURSOR);
> > > > + igt_assert(psr_wait_entry_if_enabled(&d
> > > > ata));
> > > > + run_test(&data);
> > > > + test_cleanup(&data);
> > > > + }
> > > > + }
> > > > +
> > > > igt_subtest_f("dpms") {
> > > > data.op = RENDER;
> > > > setup_test_plane(&data,
> > > > DRM_PLANE_TYPE_PRIMARY);
> > > > @@ -474,6 +550,16 @@ int main(int argc, char *argv[])
> > > > test_cleanup(&data);
> > > > }
> > > >
> > > > + igt_subtest_f("psr2_dpms") {
> > > > + data.op_psr2 = true;
> > > > + data.op = RENDER;
> > > > + setup_test_plane(&data,
> > > > DRM_PLANE_TYPE_PRIMARY);
> > > > + igt_assert(psr_wait_entry_if_enabled(&data));
> > > > + dpms_off_on(&data);
> > > > + run_test(&data);
> > > > + test_cleanup(&data);
> > > > + }
> > > > +
> > > > igt_subtest_f("suspend") {
> > > > data.op = PLANE_ONOFF;
> > > > setup_test_plane(&data, DRM_PLANE_TYPE_CURSOR);
> > > > @@ -485,6 +571,18 @@ int main(int argc, char *argv[])
> > > > test_cleanup(&data);
> > > > }
> > > >
> > > > + igt_subtest_f("psr2_suspend") {
> > > > + data.op_psr2 = true;
> > > > + data.op = PLANE_ONOFF;
> > > > + setup_test_plane(&data, DRM_PLANE_TYPE_CURSOR);
> > > > + igt_assert(psr_wait_entry_if_enabled(&data));
> > > > + igt_system_suspend_autoresume(SUSPEND_STATE_MEM
> > > > ,
> > > > + SUSPEND_TEST_NONE
> > > > );
> > > > + igt_assert(psr_wait_entry_if_enabled(&data));
> > > > + run_test(&data);
> > > > + test_cleanup(&data);
> > > > + }
> > > > +
> > > > igt_fixture {
> > > > if (!data.with_psr_disabled)
> > > > psr_disable(data.debugfs_fd);
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: This is a digitally signed message part
URL: <https://lists.freedesktop.org/archives/igt-dev/attachments/20190111/a33e0a9d/attachment-0001.sig>
More information about the igt-dev
mailing list