[igt-dev] [PATCH i-g-t 6/8] tests/psr: Add the same test coverage that we have for PSR1 to PSR2
Souza, Jose
jose.souza at intel.com
Thu Dec 13 19:55:56 UTC 2018
On Tue, 2018-12-11 at 17:20 -0800, Dhinakaran Pandiyan wrote:
> On Tue, 2018-12-04 at 15:09 -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 | 118
> > +++++++++++++++++++++++++++++++++++++++++++++-
> > --
> > 1 file changed, 111 insertions(+), 7 deletions(-)
> >
> > diff --git a/tests/kms_psr.c b/tests/kms_psr.c
> > index 116fe409..d87b8ea1 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,8 @@ typedef struct {
> > drmModeModeInfo *mode;
> > igt_output_t *output;
> > bool with_psr_disabled;
> > + bool with_psr2_disabled;
> Do we need this? Is there a use case running PSR2 tests with PSR1
> enabled?
Huum yeah, I will drop the with_psr2_disabled.
>
>
> > + bool supports_psr2;
> > } data_t;
> >
> > static void create_cursor_fb(data_t *data)
> > @@ -194,12 +197,22 @@ static bool sink_support(data_t *data)
> > return data->with_psr_disabled || psr_supported(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_psr2_disabled || psr2_supported(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_psr2_disabled)
> > + return true;
> > + return psr2_wait_deep_sleep(data->debugfs_fd);
> > + }
> > }
> >
> > static inline void manual(const char *expected)
> > @@ -289,11 +302,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));
> Move the PSR1/PSR2 logic inside psr_wait_exit() ?
Again I would like to keep psr and psr2 functions, it is more easy to
read.
>
> > 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 +322,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 +334,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);//op_psr2 is set in
> > case this fails?
> > + psr2_enable(data->debugfs_fd);
> > + }
> > +
> > igt_create_color_fb(data->drm_fd,
> > data->mode->hdisplay, data->mode->vdisplay,
> > DRM_FORMAT_XRGB8888,
> > @@ -381,6 +409,9 @@ static int opt_handler(int opt, int opt_index,
> > void *_data)
> > case 'n':
> > data->with_psr_disabled = true;
> > break;
> > + case 'm':
> > + data->with_psr2_disabled = true;
> > + break;
> > default:
> > igt_assert(0);
> > }
> > @@ -391,9 +422,11 @@ 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.\n"
> > + " --no-psr2\tRun test without PSR2.";
> > static struct option long_options[] = {
> > {"no-psr", 0, 0, 'n'},
> > + {"no-psr2", 0, 0, 'm'},
> > { 0, 0, 0, 0 }
> > };
> > data_t data = {};
> > @@ -414,6 +447,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 +462,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 +476,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 +494,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(&data));
> > + 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 +515,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(&data));
> > + 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 +536,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(&data));
> > + run_test(&data);
> > + test_cleanup(&data);
> > + }
> > + }
> > +
> > igt_subtest_f("dpms") {
> > data.op = RENDER;
> > setup_test_plane(&data, DRM_PLANE_TYPE_PRIMARY);
> > @@ -474,6 +556,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 +577,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);
> > + }
> > +
> How about using a loop to iterate PSR1 and PSR2 modes?
Having a test for PSR and PSR2 is better to keep track of bug and for
us to fix, so we know for sure in what PSR version it is failling.
> > 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/20181213/85fb5733/attachment.sig>
More information about the igt-dev
mailing list