* [igt-dev] [PATCH i-g-t] tests/kms_dp_dsc: Force a full modeset when we force dsc enable @ 2019-04-02 19:31 Manasi Navare 2019-04-02 20:10 ` [igt-dev] ✓ Fi.CI.BAT: success for " Patchwork ` (4 more replies) 0 siblings, 5 replies; 19+ messages in thread From: Manasi Navare @ 2019-04-02 19:31 UTC (permalink / raw) To: igt-dev; +Cc: Manasi Navare, Anusha Srivatsa, Petri Latvala DSC enable gets configured during compute_config and needs a full modeset to force DSC. Sometimes in between the tests, if the initial output is same as the mode being set for DSC then it will not do a full modeset. So we disable the output before forcing a mode with DSC enable. Fixes: db19bccc1c22 ("test/kms_dp_dsc: Basic KMS test to validate VESA DSC on DP/eDP") Cc: Petri Latvala <petri.latvala@intel.com> Cc: Anusha Srivatsa <anusha.srivatsa@intel.com> Cc: Imre Deak <imre.deak@intel.com> Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> --- tests/kms_dp_dsc.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/kms_dp_dsc.c b/tests/kms_dp_dsc.c index da93cd74..f3a61029 100644 --- a/tests/kms_dp_dsc.c +++ b/tests/kms_dp_dsc.c @@ -174,6 +174,11 @@ static void update_display(data_t *data, enum dsc_test_type test_type) &data->fb_test_pattern); primary = igt_output_get_plane_type(data->output, DRM_PLANE_TYPE_PRIMARY); + /* Disable the output first */ + igt_plane_set_fb(primary, NULL); + igt_output_set_pipe(data->output, PIPE_NONE); + + /* Now set the output to the desired mode */ igt_plane_set_fb(primary, &data->fb_test_pattern); igt_display_commit(&data->display); @@ -187,7 +192,7 @@ static void update_display(data_t *data, enum dsc_test_type test_type) clear_dp_dsc_enable(data); igt_assert_f(enabled, - "Default DSC enable failed on Connector: %s Pipe: %s", + "\nDefault DSC enable failed on Connector: %s Pipe: %s", data->conn_name, kmstest_pipe_name(data->pipe)); } else { -- 2.19.1 _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [igt-dev] ✓ Fi.CI.BAT: success for tests/kms_dp_dsc: Force a full modeset when we force dsc enable 2019-04-02 19:31 [igt-dev] [PATCH i-g-t] tests/kms_dp_dsc: Force a full modeset when we force dsc enable Manasi Navare @ 2019-04-02 20:10 ` Patchwork 2019-04-03 7:46 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork ` (3 subsequent siblings) 4 siblings, 0 replies; 19+ messages in thread From: Patchwork @ 2019-04-02 20:10 UTC (permalink / raw) To: Manasi Navare; +Cc: igt-dev == Series Details == Series: tests/kms_dp_dsc: Force a full modeset when we force dsc enable URL : https://patchwork.freedesktop.org/series/58887/ State : success == Summary == CI Bug Log - changes from CI_DRM_5856 -> IGTPW_2766 ==================================================== Summary ------- **SUCCESS** No regressions found. External URL: https://patchwork.freedesktop.org/api/1.0/series/58887/revisions/1/mbox/ Known issues ------------ Here are the changes found in IGTPW_2766 that come from known issues: ### IGT changes ### #### Issues hit #### * igt@amdgpu/amd_cs_nop@fork-compute0: - fi-icl-y: NOTRUN -> SKIP [fdo#109315] +17 * igt@amdgpu/amd_cs_nop@fork-gfx0: - fi-icl-u2: NOTRUN -> SKIP [fdo#109315] +17 * igt@gem_exec_basic@basic-bsd2: - fi-kbl-7500u: NOTRUN -> SKIP [fdo#109271] +9 - fi-icl-y: NOTRUN -> SKIP [fdo#109276] +7 * igt@gem_exec_basic@readonly-bsd1: - fi-icl-u2: NOTRUN -> SKIP [fdo#109276] +7 * igt@gem_exec_parse@basic-allowed: - fi-icl-u2: NOTRUN -> SKIP [fdo#109289] +1 * igt@gem_exec_parse@basic-rejected: - fi-icl-y: NOTRUN -> SKIP [fdo#109289] +1 * igt@i915_selftest@live_contexts: - fi-icl-u2: NOTRUN -> DMESG-FAIL [fdo#108569] - fi-icl-y: NOTRUN -> DMESG-FAIL [fdo#108569] * igt@i915_selftest@live_execlists: - fi-apl-guc: NOTRUN -> INCOMPLETE [fdo#103927] / [fdo#109720] * igt@kms_chamelium@dp-crc-fast: - fi-kbl-7500u: NOTRUN -> DMESG-WARN [fdo#103841] - fi-icl-y: NOTRUN -> SKIP [fdo#109284] +8 * igt@kms_chamelium@dp-edid-read: - fi-icl-u2: NOTRUN -> SKIP [fdo#109316] +2 * igt@kms_chamelium@vga-hpd-fast: - fi-icl-u2: NOTRUN -> SKIP [fdo#109309] +1 * igt@kms_force_connector_basic@force-load-detect: - fi-icl-y: NOTRUN -> SKIP [fdo#109285] +3 * igt@kms_force_connector_basic@prune-stale-modes: - fi-icl-u2: NOTRUN -> SKIP [fdo#109285] +3 * igt@kms_frontbuffer_tracking@basic: - fi-icl-u2: NOTRUN -> FAIL [fdo#103167] * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a: - fi-byt-clapper: PASS -> FAIL [fdo#107362] * igt@kms_pipe_crc_basic@read-crc-pipe-a-frame-sequence: - fi-byt-clapper: PASS -> FAIL [fdo#103191] / [fdo#107362] * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b: - fi-blb-e6850: PASS -> INCOMPLETE [fdo#107718] * igt@kms_psr@primary_mmap_gtt: - fi-icl-y: NOTRUN -> SKIP [fdo#110189] +3 - fi-byt-clapper: NOTRUN -> SKIP [fdo#109271] +23 * igt@kms_psr@primary_page_flip: - fi-apl-guc: NOTRUN -> SKIP [fdo#109271] +50 * igt@prime_vgem@basic-fence-flip: - fi-icl-y: NOTRUN -> SKIP [fdo#109294] * igt@runner@aborted: - fi-kbl-7500u: NOTRUN -> FAIL [fdo#103841] - fi-apl-guc: NOTRUN -> FAIL [fdo#108622] / [fdo#109720] #### Possible fixes #### * igt@i915_selftest@live_contexts: - fi-bdw-gvtdvm: DMESG-FAIL [fdo#110235 ] -> PASS - fi-skl-gvtdvm: DMESG-FAIL [fdo#110235 ] -> PASS * igt@i915_selftest@live_uncore: - fi-skl-gvtdvm: DMESG-FAIL [fdo#110210] -> PASS * igt@kms_frontbuffer_tracking@basic: - fi-icl-u3: FAIL [fdo#103167] -> PASS - fi-byt-clapper: FAIL [fdo#103167] -> PASS * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b: - fi-byt-clapper: FAIL [fdo#107362] -> PASS * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a: - fi-byt-clapper: INCOMPLETE [fdo#102657] -> PASS #### Warnings #### * igt@i915_selftest@live_contexts: - fi-icl-u3: INCOMPLETE [fdo#108569] -> DMESG-FAIL [fdo#108569] [fdo#102657]: https://bugs.freedesktop.org/show_bug.cgi?id=102657 [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167 [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191 [fdo#103841]: https://bugs.freedesktop.org/show_bug.cgi?id=103841 [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927 [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362 [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718 [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569 [fdo#108622]: https://bugs.freedesktop.org/show_bug.cgi?id=108622 [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271 [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276 [fdo#109284]: https://bugs.freedesktop.org/show_bug.cgi?id=109284 [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285 [fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289 [fdo#109294]: https://bugs.freedesktop.org/show_bug.cgi?id=109294 [fdo#109309]: https://bugs.freedesktop.org/show_bug.cgi?id=109309 [fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315 [fdo#109316]: https://bugs.freedesktop.org/show_bug.cgi?id=109316 [fdo#109720]: https://bugs.freedesktop.org/show_bug.cgi?id=109720 [fdo#110189]: https://bugs.freedesktop.org/show_bug.cgi?id=110189 [fdo#110210]: https://bugs.freedesktop.org/show_bug.cgi?id=110210 [fdo#110235 ]: https://bugs.freedesktop.org/show_bug.cgi?id=110235 Participating hosts (43 -> 43) ------------------------------ Additional (4): fi-icl-y fi-icl-u2 fi-apl-guc fi-kbl-7500u Missing (4): fi-ilk-m540 fi-byt-squawks fi-skl-6260u fi-bdw-samus Build changes ------------- * IGT: IGT_4922 -> IGTPW_2766 CI_DRM_5856: 55074bd825098a71779cf65a69786547f0eccbe9 @ git://anongit.freedesktop.org/gfx-ci/linux IGTPW_2766: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2766/ IGT_4922: e941e4a29438c7130554492e4daf52afbc99ffdf @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2766/ _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 19+ messages in thread
* [igt-dev] ✓ Fi.CI.IGT: success for tests/kms_dp_dsc: Force a full modeset when we force dsc enable 2019-04-02 19:31 [igt-dev] [PATCH i-g-t] tests/kms_dp_dsc: Force a full modeset when we force dsc enable Manasi Navare 2019-04-02 20:10 ` [igt-dev] ✓ Fi.CI.BAT: success for " Patchwork @ 2019-04-03 7:46 ` Patchwork 2019-04-03 10:06 ` [igt-dev] [PATCH i-g-t] " Petri Latvala ` (2 subsequent siblings) 4 siblings, 0 replies; 19+ messages in thread From: Patchwork @ 2019-04-03 7:46 UTC (permalink / raw) To: Manasi Navare; +Cc: igt-dev == Series Details == Series: tests/kms_dp_dsc: Force a full modeset when we force dsc enable URL : https://patchwork.freedesktop.org/series/58887/ State : success == Summary == CI Bug Log - changes from CI_DRM_5856_full -> IGTPW_2766_full ==================================================== Summary ------- **SUCCESS** No regressions found. External URL: https://patchwork.freedesktop.org/api/1.0/series/58887/revisions/1/mbox/ Known issues ------------ Here are the changes found in IGTPW_2766_full that come from known issues: ### IGT changes ### #### Issues hit #### * igt@gem_exec_store@cachelines-bsd2: - shard-glk: NOTRUN -> SKIP [fdo#109271] +20 * igt@i915_pm_rpm@system-suspend-modeset: - shard-kbl: PASS -> INCOMPLETE [fdo#103665] / [fdo#107807] * igt@kms_atomic_transition@1x-modeset-transitions-nonblocking: - shard-apl: PASS -> FAIL [fdo#109660] * igt@kms_atomic_transition@3x-modeset-transitions: - shard-hsw: NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +3 * igt@kms_atomic_transition@plane-all-modeset-transition-fencing: - shard-apl: PASS -> INCOMPLETE [fdo#103927] * igt@kms_available_modes_crc@available_mode_test_crc: - shard-snb: NOTRUN -> FAIL [fdo#106641] * igt@kms_busy@basic-modeset-e: - shard-kbl: NOTRUN -> SKIP [fdo#109271] / [fdo#109278] * igt@kms_busy@extended-modeset-hang-newfb-render-a: - shard-hsw: PASS -> DMESG-WARN [fdo#110222] +2 * igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-c: - shard-kbl: PASS -> DMESG-WARN [fdo#110222] * igt@kms_chamelium@common-hpd-after-suspend: - shard-hsw: NOTRUN -> SKIP [fdo#109271] +21 * igt@kms_cursor_crc@cursor-256x256-suspend: - shard-kbl: PASS -> FAIL [fdo#103191] / [fdo#103232] - shard-apl: PASS -> FAIL [fdo#103191] / [fdo#103232] * igt@kms_cursor_legacy@cursor-vs-flip-toggle: - shard-glk: PASS -> INCOMPLETE [fdo#103359] / [k.org#198133] * igt@kms_flip@2x-wf_vblank-ts-check: - shard-snb: NOTRUN -> SKIP [fdo#109271] +68 * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-blt: - shard-apl: PASS -> FAIL [fdo#103167] - shard-kbl: PASS -> FAIL [fdo#103167] * igt@kms_frontbuffer_tracking@fbcpsr-2p-primscrn-cur-indfb-draw-mmap-wc: - shard-kbl: NOTRUN -> SKIP [fdo#109271] +23 * igt@kms_lease@atomic_implicit_crtc: - shard-kbl: NOTRUN -> FAIL [fdo#110279] * igt@kms_plane_alpha_blend@pipe-a-alpha-7efc: - shard-kbl: NOTRUN -> FAIL [fdo#108145] / [fdo#108590] * igt@kms_plane_scaling@pipe-a-scaler-with-clipping-clamping: - shard-glk: PASS -> SKIP [fdo#109271] / [fdo#109278] +1 * igt@kms_plane_scaling@pipe-b-scaler-with-clipping-clamping: - shard-glk: NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +2 * igt@kms_rotation_crc@multiplane-rotation-cropping-top: - shard-kbl: PASS -> FAIL [fdo#109016] +1 * igt@kms_setmode@basic: - shard-kbl: PASS -> FAIL [fdo#99912] * igt@kms_vblank@pipe-a-ts-continuation-dpms-rpm: - shard-snb: NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +5 - shard-kbl: PASS -> FAIL [fdo#104894] * igt@kms_vblank@pipe-b-ts-continuation-dpms-rpm: - shard-apl: PASS -> FAIL [fdo#104894] +3 #### Possible fixes #### * igt@gem_create@create-clear: - shard-snb: INCOMPLETE [fdo#105411] -> PASS * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-a: - shard-hsw: DMESG-WARN [fdo#110222] -> PASS * igt@kms_cursor_crc@cursor-64x64-sliding: - shard-glk: FAIL [fdo#103232] -> PASS +1 * igt@kms_cursor_legacy@2x-long-nonblocking-modeset-vs-cursor-atomic: - shard-glk: FAIL [fdo#106509] / [fdo#107409] -> PASS * igt@kms_plane@pixel-format-pipe-c-planes-source-clamping: - shard-glk: SKIP [fdo#109271] -> PASS * igt@kms_vblank@pipe-a-ts-continuation-dpms-suspend: - shard-kbl: INCOMPLETE [fdo#103665] -> PASS * igt@kms_vblank@pipe-c-ts-continuation-dpms-suspend: - shard-apl: FAIL [fdo#104894] -> PASS #### Warnings #### * igt@gem_tiled_swapping@non-threaded: - shard-hsw: FAIL [fdo#108686] -> INCOMPLETE [fdo#103540] * igt@kms_frontbuffer_tracking@psr-2p-primscrn-pri-shrfb-draw-render: - shard-hsw: INCOMPLETE [fdo#103540] -> SKIP [fdo#109271] [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167 [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191 [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232 [fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359 [fdo#103540]: https://bugs.freedesktop.org/show_bug.cgi?id=103540 [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665 [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927 [fdo#104894]: https://bugs.freedesktop.org/show_bug.cgi?id=104894 [fdo#105411]: https://bugs.freedesktop.org/show_bug.cgi?id=105411 [fdo#106509]: https://bugs.freedesktop.org/show_bug.cgi?id=106509 [fdo#106641]: https://bugs.freedesktop.org/show_bug.cgi?id=106641 [fdo#107409]: https://bugs.freedesktop.org/show_bug.cgi?id=107409 [fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807 [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145 [fdo#108590]: https://bugs.freedesktop.org/show_bug.cgi?id=108590 [fdo#108686]: https://bugs.freedesktop.org/show_bug.cgi?id=108686 [fdo#109016]: https://bugs.freedesktop.org/show_bug.cgi?id=109016 [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271 [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278 [fdo#109660]: https://bugs.freedesktop.org/show_bug.cgi?id=109660 [fdo#110222]: https://bugs.freedesktop.org/show_bug.cgi?id=110222 [fdo#110279]: https://bugs.freedesktop.org/show_bug.cgi?id=110279 [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912 [k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133 Participating hosts (10 -> 5) ------------------------------ Missing (5): shard-skl pig-hsw-4770r pig-glk-j5005 shard-iclb pig-skl-6260u Build changes ------------- * IGT: IGT_4922 -> IGTPW_2766 * Piglit: piglit_4509 -> None CI_DRM_5856: 55074bd825098a71779cf65a69786547f0eccbe9 @ git://anongit.freedesktop.org/gfx-ci/linux IGTPW_2766: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2766/ IGT_4922: e941e4a29438c7130554492e4daf52afbc99ffdf @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2766/ _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [igt-dev] [PATCH i-g-t] tests/kms_dp_dsc: Force a full modeset when we force dsc enable 2019-04-02 19:31 [igt-dev] [PATCH i-g-t] tests/kms_dp_dsc: Force a full modeset when we force dsc enable Manasi Navare 2019-04-02 20:10 ` [igt-dev] ✓ Fi.CI.BAT: success for " Patchwork 2019-04-03 7:46 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork @ 2019-04-03 10:06 ` Petri Latvala 2019-04-03 13:25 ` Imre Deak 2019-04-03 17:38 ` Srivatsa, Anusha 4 siblings, 0 replies; 19+ messages in thread From: Petri Latvala @ 2019-04-03 10:06 UTC (permalink / raw) To: Manasi Navare; +Cc: igt-dev, Anusha Srivatsa On Tue, Apr 02, 2019 at 12:31:19PM -0700, Manasi Navare wrote: > DSC enable gets configured during compute_config and needs > a full modeset to force DSC. > Sometimes in between the tests, if the initial output is same as the > mode being set for DSC then it will not do a full modeset. > So we disable the output before forcing a mode with DSC enable. > > Fixes: db19bccc1c22 ("test/kms_dp_dsc: Basic KMS test to validate VESA DSC on DP/eDP") > Cc: Petri Latvala <petri.latvala@intel.com> > Cc: Anusha Srivatsa <anusha.srivatsa@intel.com> > Cc: Imre Deak <imre.deak@intel.com> > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > --- > tests/kms_dp_dsc.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/tests/kms_dp_dsc.c b/tests/kms_dp_dsc.c > index da93cd74..f3a61029 100644 > --- a/tests/kms_dp_dsc.c > +++ b/tests/kms_dp_dsc.c > @@ -174,6 +174,11 @@ static void update_display(data_t *data, enum dsc_test_type test_type) > &data->fb_test_pattern); > primary = igt_output_get_plane_type(data->output, > DRM_PLANE_TYPE_PRIMARY); > + /* Disable the output first */ > + igt_plane_set_fb(primary, NULL); > + igt_output_set_pipe(data->output, PIPE_NONE); > + > + /* Now set the output to the desired mode */ > igt_plane_set_fb(primary, &data->fb_test_pattern); > igt_display_commit(&data->display); > > @@ -187,7 +192,7 @@ static void update_display(data_t *data, enum dsc_test_type test_type) > clear_dp_dsc_enable(data); > > igt_assert_f(enabled, > - "Default DSC enable failed on Connector: %s Pipe: %s", > + "\nDefault DSC enable failed on Connector: %s Pipe: %s", ??? Did you mean to put that \n at the end of the string? As Arek did in https://patchwork.freedesktop.org/series/58831/ -- Petri Latvala _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [igt-dev] [PATCH i-g-t] tests/kms_dp_dsc: Force a full modeset when we force dsc enable 2019-04-02 19:31 [igt-dev] [PATCH i-g-t] tests/kms_dp_dsc: Force a full modeset when we force dsc enable Manasi Navare ` (2 preceding siblings ...) 2019-04-03 10:06 ` [igt-dev] [PATCH i-g-t] " Petri Latvala @ 2019-04-03 13:25 ` Imre Deak 2019-04-03 18:20 ` Manasi Navare 2019-04-03 17:38 ` Srivatsa, Anusha 4 siblings, 1 reply; 19+ messages in thread From: Imre Deak @ 2019-04-03 13:25 UTC (permalink / raw) To: Manasi Navare; +Cc: igt-dev, Anusha Srivatsa, Petri Latvala On Tue, Apr 02, 2019 at 12:31:19PM -0700, Manasi Navare wrote: > DSC enable gets configured during compute_config and needs > a full modeset to force DSC. > Sometimes in between the tests, if the initial output is same as the > mode being set for DSC then it will not do a full modeset. > So we disable the output before forcing a mode with DSC enable. > > Fixes: db19bccc1c22 ("test/kms_dp_dsc: Basic KMS test to validate VESA DSC on DP/eDP") > Cc: Petri Latvala <petri.latvala@intel.com> > Cc: Anusha Srivatsa <anusha.srivatsa@intel.com> > Cc: Imre Deak <imre.deak@intel.com> > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > --- > tests/kms_dp_dsc.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/tests/kms_dp_dsc.c b/tests/kms_dp_dsc.c > index da93cd74..f3a61029 100644 > --- a/tests/kms_dp_dsc.c > +++ b/tests/kms_dp_dsc.c > @@ -174,6 +174,11 @@ static void update_display(data_t *data, enum dsc_test_type test_type) > &data->fb_test_pattern); > primary = igt_output_get_plane_type(data->output, > DRM_PLANE_TYPE_PRIMARY); > + /* Disable the output first */ > + igt_plane_set_fb(primary, NULL); I think no need to reset the plane, it's enough to set PIPE_NONE to turn off the output. > + igt_output_set_pipe(data->output, PIPE_NONE); You are missing here the additional commit needed to turn off the output and then set back the output to data->pipe before the commit below. The igt_output_set_pipe() call in run_test() won't be needed either. > + > + /* Now set the output to the desired mode */ > igt_plane_set_fb(primary, &data->fb_test_pattern); > igt_display_commit(&data->display); > > @@ -187,7 +192,7 @@ static void update_display(data_t *data, enum dsc_test_type test_type) > clear_dp_dsc_enable(data); Instead of this we should restore the original value of i915_dsc_fec_support and also make sure we restore it if the test fails and aborts somewhere before this call. Take a look at a look at the various places changing debugfs entries that call igt_install_exit_handler() for an idea how to do this. > > igt_assert_f(enabled, > - "Default DSC enable failed on Connector: %s Pipe: %s", > + "\nDefault DSC enable failed on Connector: %s Pipe: %s", > data->conn_name, > kmstest_pipe_name(data->pipe)); > } else { > -- > 2.19.1 > _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [igt-dev] [PATCH i-g-t] tests/kms_dp_dsc: Force a full modeset when we force dsc enable 2019-04-03 13:25 ` Imre Deak @ 2019-04-03 18:20 ` Manasi Navare 2019-04-03 19:33 ` Imre Deak 0 siblings, 1 reply; 19+ messages in thread From: Manasi Navare @ 2019-04-03 18:20 UTC (permalink / raw) To: Imre Deak; +Cc: igt-dev, Anusha Srivatsa, Petri Latvala On Wed, Apr 03, 2019 at 04:25:10PM +0300, Imre Deak wrote: > On Tue, Apr 02, 2019 at 12:31:19PM -0700, Manasi Navare wrote: > > DSC enable gets configured during compute_config and needs > > a full modeset to force DSC. > > Sometimes in between the tests, if the initial output is same as the > > mode being set for DSC then it will not do a full modeset. > > So we disable the output before forcing a mode with DSC enable. > > > > Fixes: db19bccc1c22 ("test/kms_dp_dsc: Basic KMS test to validate VESA DSC on DP/eDP") > > Cc: Petri Latvala <petri.latvala@intel.com> > > Cc: Anusha Srivatsa <anusha.srivatsa@intel.com> > > Cc: Imre Deak <imre.deak@intel.com> > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > > --- > > tests/kms_dp_dsc.c | 7 ++++++- > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > diff --git a/tests/kms_dp_dsc.c b/tests/kms_dp_dsc.c > > index da93cd74..f3a61029 100644 > > --- a/tests/kms_dp_dsc.c > > +++ b/tests/kms_dp_dsc.c > > @@ -174,6 +174,11 @@ static void update_display(data_t *data, enum dsc_test_type test_type) > > &data->fb_test_pattern); > > primary = igt_output_get_plane_type(data->output, > > DRM_PLANE_TYPE_PRIMARY); > > + /* Disable the output first */ > > + igt_plane_set_fb(primary, NULL); > > I think no need to reset the plane, it's enough to set PIPE_NONE to turn > off the output. > > > + igt_output_set_pipe(data->output, PIPE_NONE); > > You are missing here the additional commit needed to turn off the output > and then set back the output to data->pipe before the commit below. The > igt_output_set_pipe() call in run_test() won't be needed either. Oh I looked through all the other kms tests and there we always set the fb for PRIMARY plane to NULL then set the pipe to NULL. But I guess if we disable to pipe output, the plane if its NULL or not shouldnt really matter. So I should just have: igt_plane_set_fb(primary, NULL); igt_display_commit(&data->display) Then set fb to the test pattern and display commit to set output to the desired mode corerct? So why is it that I wont need the igt_output_set_pipe(output, pipe) in run_test()? I see that all the kms tests use this call for_each_valid_output_on_pipe iterator > > > + > > + /* Now set the output to the desired mode */ > > igt_plane_set_fb(primary, &data->fb_test_pattern); > > igt_display_commit(&data->display); > > > > @@ -187,7 +192,7 @@ static void update_display(data_t *data, enum dsc_test_type test_type) > > clear_dp_dsc_enable(data); > > Instead of this we should restore the original value of > i915_dsc_fec_support and also make sure we restore it if the test fails > and aborts somewhere before this call. Take a look at a look at the > various places changing debugfs entries that call > igt_install_exit_handler() for an idea how to do this. So save the original value of i915_dsc_fec_support() and restore that in test_cleanup() and in the igt_fixture(), have: kmstest_set_vt_graphics_mode(); igt_install_exit_handler() Manasi > > > > > igt_assert_f(enabled, > > - "Default DSC enable failed on Connector: %s Pipe: %s", > > + "\nDefault DSC enable failed on Connector: %s Pipe: %s", > > data->conn_name, > > kmstest_pipe_name(data->pipe)); > > } else { > > -- > > 2.19.1 > > _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [igt-dev] [PATCH i-g-t] tests/kms_dp_dsc: Force a full modeset when we force dsc enable 2019-04-03 18:20 ` Manasi Navare @ 2019-04-03 19:33 ` Imre Deak 2019-04-03 20:17 ` Manasi Navare 0 siblings, 1 reply; 19+ messages in thread From: Imre Deak @ 2019-04-03 19:33 UTC (permalink / raw) To: Manasi Navare; +Cc: igt-dev, Anusha Srivatsa, Petri Latvala On Wed, Apr 03, 2019 at 11:20:58AM -0700, Manasi Navare wrote: > On Wed, Apr 03, 2019 at 04:25:10PM +0300, Imre Deak wrote: > > On Tue, Apr 02, 2019 at 12:31:19PM -0700, Manasi Navare wrote: > > > DSC enable gets configured during compute_config and needs > > > a full modeset to force DSC. > > > Sometimes in between the tests, if the initial output is same as the > > > mode being set for DSC then it will not do a full modeset. > > > So we disable the output before forcing a mode with DSC enable. > > > > > > Fixes: db19bccc1c22 ("test/kms_dp_dsc: Basic KMS test to validate VESA DSC on DP/eDP") > > > Cc: Petri Latvala <petri.latvala@intel.com> > > > Cc: Anusha Srivatsa <anusha.srivatsa@intel.com> > > > Cc: Imre Deak <imre.deak@intel.com> > > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > > > --- > > > tests/kms_dp_dsc.c | 7 ++++++- > > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > > > diff --git a/tests/kms_dp_dsc.c b/tests/kms_dp_dsc.c > > > index da93cd74..f3a61029 100644 > > > --- a/tests/kms_dp_dsc.c > > > +++ b/tests/kms_dp_dsc.c > > > @@ -174,6 +174,11 @@ static void update_display(data_t *data, enum dsc_test_type test_type) > > > &data->fb_test_pattern); > > > primary = igt_output_get_plane_type(data->output, > > > DRM_PLANE_TYPE_PRIMARY); > > > + /* Disable the output first */ > > > + igt_plane_set_fb(primary, NULL); > > > > I think no need to reset the plane, it's enough to set PIPE_NONE to turn > > off the output. > > > > > + igt_output_set_pipe(data->output, PIPE_NONE); > > > > You are missing here the additional commit needed to turn off the output > > and then set back the output to data->pipe before the commit below. The > > igt_output_set_pipe() call in run_test() won't be needed either. > > Oh I looked through all the other kms tests and there we always set > the fb for PRIMARY plane to NULL then set the pipe to NULL. But I > guess if we disable to pipe output, the plane if its NULL or not > shouldnt really matter. So I should just have: > > igt_plane_set_fb(primary, NULL); > igt_display_commit(&data->display); You need to set PIPE_NONE before the commit and then the actual pipe before the rest of the sequence, like: igt_debug("DSC is supported on %s\n", data->conn_name); igt_output_set_pipe(data->output, PIPE_NONE); igt_display_commit(&data->display); force_dp_dsc_enable(...); igt_output_set_pipe(data->output, data->pipe); igt_create_pattern_fb(...); primary = ... igt_plane_set_fb(...); igt_display_commit(&data->display); > Then set fb to the test pattern and display commit to set output to > the desired mode corerct? > > So why is it that I wont need the igt_output_set_pipe(output, pipe) in > run_test()? I see that all the kms tests use this call > for_each_valid_output_on_pipe iterator Because you will anyway set it to PIPE_NONE right afterwards in update_display(). > > > + > > > + /* Now set the output to the desired mode */ > > > igt_plane_set_fb(primary, &data->fb_test_pattern); > > > igt_display_commit(&data->display); > > > > > > @@ -187,7 +192,7 @@ static void update_display(data_t *data, enum dsc_test_type test_type) > > > clear_dp_dsc_enable(data); > > > > Instead of this we should restore the original value of > > i915_dsc_fec_support and also make sure we restore it if the test fails > > and aborts somewhere before this call. Take a look at a look at the > > various places changing debugfs entries that call > > igt_install_exit_handler() for an idea how to do this. > > So save the original value of i915_dsc_fec_support() and restore that > in test_cleanup() Save the original value in force_dp_dsc_enable() and restore it in a new restore_dp_dsc_enable() which you call instead of clear_dp_dsc_enable() in update_display(). > and in the igt_fixture(), have: > kmstest_set_vt_graphics_mode(); > igt_install_exit_handler() The exit handler should be installed only after saving the original value, so the handler will have the proper value to restore. Save/install the exit handler only once even if force_dp_dsc_enable() is called multiple times. > > Manasi > > > > > > > > > igt_assert_f(enabled, > > > - "Default DSC enable failed on Connector: %s Pipe: %s", > > > + "\nDefault DSC enable failed on Connector: %s Pipe: %s", > > > data->conn_name, > > > kmstest_pipe_name(data->pipe)); > > > } else { > > > -- > > > 2.19.1 > > > _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [igt-dev] [PATCH i-g-t] tests/kms_dp_dsc: Force a full modeset when we force dsc enable 2019-04-03 19:33 ` Imre Deak @ 2019-04-03 20:17 ` Manasi Navare 2019-04-03 20:34 ` Imre Deak 0 siblings, 1 reply; 19+ messages in thread From: Manasi Navare @ 2019-04-03 20:17 UTC (permalink / raw) To: Imre Deak; +Cc: igt-dev, Anusha Srivatsa, Petri Latvala On Wed, Apr 03, 2019 at 10:33:41PM +0300, Imre Deak wrote: > On Wed, Apr 03, 2019 at 11:20:58AM -0700, Manasi Navare wrote: > > On Wed, Apr 03, 2019 at 04:25:10PM +0300, Imre Deak wrote: > > > On Tue, Apr 02, 2019 at 12:31:19PM -0700, Manasi Navare wrote: > > > > DSC enable gets configured during compute_config and needs > > > > a full modeset to force DSC. > > > > Sometimes in between the tests, if the initial output is same as the > > > > mode being set for DSC then it will not do a full modeset. > > > > So we disable the output before forcing a mode with DSC enable. > > > > > > > > Fixes: db19bccc1c22 ("test/kms_dp_dsc: Basic KMS test to validate VESA DSC on DP/eDP") > > > > Cc: Petri Latvala <petri.latvala@intel.com> > > > > Cc: Anusha Srivatsa <anusha.srivatsa@intel.com> > > > > Cc: Imre Deak <imre.deak@intel.com> > > > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > > > > --- > > > > tests/kms_dp_dsc.c | 7 ++++++- > > > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/tests/kms_dp_dsc.c b/tests/kms_dp_dsc.c > > > > index da93cd74..f3a61029 100644 > > > > --- a/tests/kms_dp_dsc.c > > > > +++ b/tests/kms_dp_dsc.c > > > > @@ -174,6 +174,11 @@ static void update_display(data_t *data, enum dsc_test_type test_type) > > > > &data->fb_test_pattern); > > > > primary = igt_output_get_plane_type(data->output, > > > > DRM_PLANE_TYPE_PRIMARY); > > > > + /* Disable the output first */ > > > > + igt_plane_set_fb(primary, NULL); > > > > > > I think no need to reset the plane, it's enough to set PIPE_NONE to turn > > > off the output. > > > > > > > + igt_output_set_pipe(data->output, PIPE_NONE); > > > > > > You are missing here the additional commit needed to turn off the output > > > and then set back the output to data->pipe before the commit below. The > > > igt_output_set_pipe() call in run_test() won't be needed either. > > > > Oh I looked through all the other kms tests and there we always set > > the fb for PRIMARY plane to NULL then set the pipe to NULL. But I > > guess if we disable to pipe output, the plane if its NULL or not > > shouldnt really matter. So I should just have: > > > > igt_plane_set_fb(primary, NULL); > > igt_display_commit(&data->display); > > You need to set PIPE_NONE before the commit and then the actual pipe > before the rest of the sequence, like: > > igt_debug("DSC is supported on %s\n", data->conn_name); > > igt_output_set_pipe(data->output, PIPE_NONE); > igt_display_commit(&data->display); > > force_dp_dsc_enable(...); > > igt_output_set_pipe(data->output, data->pipe); > igt_create_pattern_fb(...); > primary = ... > igt_plane_set_fb(...); > igt_display_commit(&data->display); Yea, got this part. Testing is in progress. > > > Then set fb to the test pattern and display commit to set output to > > the desired mode corerct? > > > > > So why is it that I wont need the igt_output_set_pipe(output, pipe) in > > run_test()? I see that all the kms tests use this call > > for_each_valid_output_on_pipe iterator > > Because you will anyway set it to PIPE_NONE right afterwards in > update_display(). Yup got it. > > > > > + > > > > + /* Now set the output to the desired mode */ > > > > igt_plane_set_fb(primary, &data->fb_test_pattern); > > > > igt_display_commit(&data->display); > > > > > > > > @@ -187,7 +192,7 @@ static void update_display(data_t *data, enum dsc_test_type test_type) > > > > clear_dp_dsc_enable(data); > > > > > > Instead of this we should restore the original value of > > > i915_dsc_fec_support and also make sure we restore it if the test fails > > > and aborts somewhere before this call. Take a look at a look at the > > > various places changing debugfs entries that call > > > igt_install_exit_handler() for an idea how to do this. > > > > > So save the original value of i915_dsc_fec_support() and restore that > > in test_cleanup() I think this part will require lot of changes in the debugfs code in the kernel and also the IGT, since now I just write a 1 to the node to force dsc enable and then a 0 to clear it and in the kernel we accept kstr_to_bool to get a value in this node to set force_dsc_enable() to true or false. But now basically the sysfs_write in IGT will need to write the entire original file that was read from i915_dsc_fec_support() and in the kernel we will need to accept the whole file and then parse the DSC_enabled field to take its value to use it for force_dsc_enable. Is all this what you are suggesting? And the exit_handler() that we install in that we call this test_cleanup()? Manasi > > Save the original value in force_dp_dsc_enable() and restore it in a new > restore_dp_dsc_enable() which you call instead of clear_dp_dsc_enable() > in update_display(). > > > and in the igt_fixture(), have: > > kmstest_set_vt_graphics_mode(); > > igt_install_exit_handler() > > The exit handler should be installed only after saving the original > value, so the handler will have the proper value to restore. Save/install > the exit handler only once even if force_dp_dsc_enable() is called > multiple times. > > > > > Manasi > > > > > > > > > > > > > igt_assert_f(enabled, > > > > - "Default DSC enable failed on Connector: %s Pipe: %s", > > > > + "\nDefault DSC enable failed on Connector: %s Pipe: %s", > > > > data->conn_name, > > > > kmstest_pipe_name(data->pipe)); > > > > } else { > > > > -- > > > > 2.19.1 > > > > _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [igt-dev] [PATCH i-g-t] tests/kms_dp_dsc: Force a full modeset when we force dsc enable 2019-04-03 20:17 ` Manasi Navare @ 2019-04-03 20:34 ` Imre Deak 2019-04-03 20:49 ` Manasi Navare 0 siblings, 1 reply; 19+ messages in thread From: Imre Deak @ 2019-04-03 20:34 UTC (permalink / raw) To: Manasi Navare; +Cc: igt-dev, Anusha Srivatsa, Petri Latvala On Wed, Apr 03, 2019 at 01:17:17PM -0700, Manasi Navare wrote: > On Wed, Apr 03, 2019 at 10:33:41PM +0300, Imre Deak wrote: > > On Wed, Apr 03, 2019 at 11:20:58AM -0700, Manasi Navare wrote: > > > On Wed, Apr 03, 2019 at 04:25:10PM +0300, Imre Deak wrote: > > > > On Tue, Apr 02, 2019 at 12:31:19PM -0700, Manasi Navare wrote: > > > > > DSC enable gets configured during compute_config and needs > > > > > a full modeset to force DSC. > > > > > Sometimes in between the tests, if the initial output is same as the > > > > > mode being set for DSC then it will not do a full modeset. > > > > > So we disable the output before forcing a mode with DSC enable. > > > > > > > > > > Fixes: db19bccc1c22 ("test/kms_dp_dsc: Basic KMS test to validate VESA DSC on DP/eDP") > > > > > Cc: Petri Latvala <petri.latvala@intel.com> > > > > > Cc: Anusha Srivatsa <anusha.srivatsa@intel.com> > > > > > Cc: Imre Deak <imre.deak@intel.com> > > > > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > > > > > --- > > > > > tests/kms_dp_dsc.c | 7 ++++++- > > > > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > > > > > > > diff --git a/tests/kms_dp_dsc.c b/tests/kms_dp_dsc.c > > > > > index da93cd74..f3a61029 100644 > > > > > --- a/tests/kms_dp_dsc.c > > > > > +++ b/tests/kms_dp_dsc.c > > > > > @@ -174,6 +174,11 @@ static void update_display(data_t *data, enum dsc_test_type test_type) > > > > > &data->fb_test_pattern); > > > > > primary = igt_output_get_plane_type(data->output, > > > > > DRM_PLANE_TYPE_PRIMARY); > > > > > + /* Disable the output first */ > > > > > + igt_plane_set_fb(primary, NULL); > > > > > > > > I think no need to reset the plane, it's enough to set PIPE_NONE to turn > > > > off the output. > > > > > > > > > + igt_output_set_pipe(data->output, PIPE_NONE); > > > > > > > > You are missing here the additional commit needed to turn off the output > > > > and then set back the output to data->pipe before the commit below. The > > > > igt_output_set_pipe() call in run_test() won't be needed either. > > > > > > Oh I looked through all the other kms tests and there we always set > > > the fb for PRIMARY plane to NULL then set the pipe to NULL. But I > > > guess if we disable to pipe output, the plane if its NULL or not > > > shouldnt really matter. So I should just have: > > > > > > igt_plane_set_fb(primary, NULL); > > > igt_display_commit(&data->display); > > > > You need to set PIPE_NONE before the commit and then the actual pipe > > before the rest of the sequence, like: > > > > igt_debug("DSC is supported on %s\n", data->conn_name); > > > > igt_output_set_pipe(data->output, PIPE_NONE); > > igt_display_commit(&data->display); > > > > force_dp_dsc_enable(...); > > > > igt_output_set_pipe(data->output, data->pipe); > > igt_create_pattern_fb(...); > > primary = ... > > igt_plane_set_fb(...); > > igt_display_commit(&data->display); > > Yea, got this part. > Testing is in progress. > > > > > > Then set fb to the test pattern and display commit to set output to > > > the desired mode corerct? > > > > > > > > So why is it that I wont need the igt_output_set_pipe(output, pipe) in > > > run_test()? I see that all the kms tests use this call > > > for_each_valid_output_on_pipe iterator > > > > Because you will anyway set it to PIPE_NONE right afterwards in > > update_display(). > > Yup got it. > > > > > > > > + > > > > > + /* Now set the output to the desired mode */ > > > > > igt_plane_set_fb(primary, &data->fb_test_pattern); > > > > > igt_display_commit(&data->display); > > > > > > > > > > @@ -187,7 +192,7 @@ static void update_display(data_t *data, enum dsc_test_type test_type) > > > > > clear_dp_dsc_enable(data); > > > > > > > > Instead of this we should restore the original value of > > > > i915_dsc_fec_support and also make sure we restore it if the test fails > > > > and aborts somewhere before this call. Take a look at a look at the > > > > various places changing debugfs entries that call > > > > igt_install_exit_handler() for an idea how to do this. > > > > > > > > So save the original value of i915_dsc_fec_support() and restore that > > > in test_cleanup() > > I think this part will require lot of changes in the debugfs code in > the kernel and also the IGT, since now I just write a 1 to the node to > force dsc enable and then a 0 to clear it and in the kernel we accept > kstr_to_bool to get a value in this node to set force_dsc_enable() to > true or false. But now basically the sysfs_write in IGT will need to > write the entire original file that was read from > i915_dsc_fec_support() and in the kernel we will need to accept the > whole file and then parse the DSC_enabled field to take its value to > use it for force_dsc_enable. > > Is all this what you are suggesting? Hm, no way to read back the actual debugfs value, that's weird. Then yes, I'd suggest adding a line to i915_dsc_fec_support_show() printing there intel_dp->force_dsc_en too. > And the exit_handler() that we install in that we call this test_cleanup()? The exit handler will be called automatically when the process exits for whatever reason. > > Manasi > > > > > Save the original value in force_dp_dsc_enable() and restore it in a new > > restore_dp_dsc_enable() which you call instead of clear_dp_dsc_enable() > > in update_display(). > > > > > and in the igt_fixture(), have: > > > kmstest_set_vt_graphics_mode(); > > > igt_install_exit_handler() > > > > The exit handler should be installed only after saving the original > > value, so the handler will have the proper value to restore. Save/install > > the exit handler only once even if force_dp_dsc_enable() is called > > multiple times. > > > > > > > > Manasi > > > > > > > > > > > > > > > > > igt_assert_f(enabled, > > > > > - "Default DSC enable failed on Connector: %s Pipe: %s", > > > > > + "\nDefault DSC enable failed on Connector: %s Pipe: %s", > > > > > data->conn_name, > > > > > kmstest_pipe_name(data->pipe)); > > > > > } else { > > > > > -- > > > > > 2.19.1 > > > > > _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [igt-dev] [PATCH i-g-t] tests/kms_dp_dsc: Force a full modeset when we force dsc enable 2019-04-03 20:34 ` Imre Deak @ 2019-04-03 20:49 ` Manasi Navare 2019-04-03 21:42 ` Imre Deak 0 siblings, 1 reply; 19+ messages in thread From: Manasi Navare @ 2019-04-03 20:49 UTC (permalink / raw) To: Imre Deak; +Cc: igt-dev, Anusha Srivatsa, Petri Latvala On Wed, Apr 03, 2019 at 11:34:05PM +0300, Imre Deak wrote: > On Wed, Apr 03, 2019 at 01:17:17PM -0700, Manasi Navare wrote: > > On Wed, Apr 03, 2019 at 10:33:41PM +0300, Imre Deak wrote: > > > On Wed, Apr 03, 2019 at 11:20:58AM -0700, Manasi Navare wrote: > > > > On Wed, Apr 03, 2019 at 04:25:10PM +0300, Imre Deak wrote: > > > > > On Tue, Apr 02, 2019 at 12:31:19PM -0700, Manasi Navare wrote: > > > > > > DSC enable gets configured during compute_config and needs > > > > > > a full modeset to force DSC. > > > > > > Sometimes in between the tests, if the initial output is same as the > > > > > > mode being set for DSC then it will not do a full modeset. > > > > > > So we disable the output before forcing a mode with DSC enable. > > > > > > > > > > > > Fixes: db19bccc1c22 ("test/kms_dp_dsc: Basic KMS test to validate VESA DSC on DP/eDP") > > > > > > Cc: Petri Latvala <petri.latvala@intel.com> > > > > > > Cc: Anusha Srivatsa <anusha.srivatsa@intel.com> > > > > > > Cc: Imre Deak <imre.deak@intel.com> > > > > > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > > > > > > --- > > > > > > tests/kms_dp_dsc.c | 7 ++++++- > > > > > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > > > > > > > > > diff --git a/tests/kms_dp_dsc.c b/tests/kms_dp_dsc.c > > > > > > index da93cd74..f3a61029 100644 > > > > > > --- a/tests/kms_dp_dsc.c > > > > > > +++ b/tests/kms_dp_dsc.c > > > > > > @@ -174,6 +174,11 @@ static void update_display(data_t *data, enum dsc_test_type test_type) > > > > > > &data->fb_test_pattern); > > > > > > primary = igt_output_get_plane_type(data->output, > > > > > > DRM_PLANE_TYPE_PRIMARY); > > > > > > + /* Disable the output first */ > > > > > > + igt_plane_set_fb(primary, NULL); > > > > > > > > > > I think no need to reset the plane, it's enough to set PIPE_NONE to turn > > > > > off the output. > > > > > > > > > > > + igt_output_set_pipe(data->output, PIPE_NONE); > > > > > > > > > > You are missing here the additional commit needed to turn off the output > > > > > and then set back the output to data->pipe before the commit below. The > > > > > igt_output_set_pipe() call in run_test() won't be needed either. > > > > > > > > Oh I looked through all the other kms tests and there we always set > > > > the fb for PRIMARY plane to NULL then set the pipe to NULL. But I > > > > guess if we disable to pipe output, the plane if its NULL or not > > > > shouldnt really matter. So I should just have: > > > > > > > > igt_plane_set_fb(primary, NULL); > > > > igt_display_commit(&data->display); > > > > > > You need to set PIPE_NONE before the commit and then the actual pipe > > > before the rest of the sequence, like: > > > > > > igt_debug("DSC is supported on %s\n", data->conn_name); > > > > > > igt_output_set_pipe(data->output, PIPE_NONE); > > > igt_display_commit(&data->display); > > > > > > force_dp_dsc_enable(...); > > > > > > igt_output_set_pipe(data->output, data->pipe); > > > igt_create_pattern_fb(...); > > > primary = ... > > > igt_plane_set_fb(...); > > > igt_display_commit(&data->display); > > > > Yea, got this part. > > Testing is in progress. > > > > > > > > > Then set fb to the test pattern and display commit to set output to > > > > the desired mode corerct? > > > > > > > > > > > So why is it that I wont need the igt_output_set_pipe(output, pipe) in > > > > run_test()? I see that all the kms tests use this call > > > > for_each_valid_output_on_pipe iterator > > > > > > Because you will anyway set it to PIPE_NONE right afterwards in > > > update_display(). > > > > Yup got it. > > > > > > > > > > > + > > > > > > + /* Now set the output to the desired mode */ > > > > > > igt_plane_set_fb(primary, &data->fb_test_pattern); > > > > > > igt_display_commit(&data->display); > > > > > > > > > > > > @@ -187,7 +192,7 @@ static void update_display(data_t *data, enum dsc_test_type test_type) > > > > > > clear_dp_dsc_enable(data); > > > > > > > > > > Instead of this we should restore the original value of > > > > > i915_dsc_fec_support and also make sure we restore it if the test fails > > > > > and aborts somewhere before this call. Take a look at a look at the > > > > > various places changing debugfs entries that call > > > > > igt_install_exit_handler() for an idea how to do this. > > > > > > > > > > > So save the original value of i915_dsc_fec_support() and restore that > > > > in test_cleanup() > > > > I think this part will require lot of changes in the debugfs code in > > the kernel and also the IGT, since now I just write a 1 to the node to > > force dsc enable and then a 0 to clear it and in the kernel we accept > > kstr_to_bool to get a value in this node to set force_dsc_enable() to > > true or false. But now basically the sysfs_write in IGT will need to > > write the entire original file that was read from > > i915_dsc_fec_support() and in the kernel we will need to accept the > > whole file and then parse the DSC_enabled field to take its value to > > use it for force_dsc_enable. > > > > Is all this what you are suggesting? > > Hm, no way to read back the actual debugfs value, that's weird. Then > yes, I'd suggest adding a line to i915_dsc_fec_support_show() printing > there intel_dp->force_dsc_en too. Currently we have inh i915_dsc_fec_support_show: DSC_enabled: DSC_sink_support: FEC_sink_support: And this node we open as a read only first and then as write node and force_dsc_enable() from IGT just writes a 1 to this, so you are saying that in the kernel _write() node, I accept this and write to force_dsc_enable entry in this file instead of rewriting the whole file? So in IGT also I will have to parse the file and write 1 for force_dsc_enable entry? I still cant figure out how the restore() function work, we write the stored_values and write for each of the entries in the file? And then in the kernel debugfs_write, get these and write back to each entry? Are there any existing examples in kernel debugfs and IGT where we do this? Manasi > > > And the exit_handler() that we install in that we call this test_cleanup()? > > The exit handler will be called automatically when the process exits for > whatever reason. > > > > > Manasi > > > > > > > > Save the original value in force_dp_dsc_enable() and restore it in a new > > > restore_dp_dsc_enable() which you call instead of clear_dp_dsc_enable() > > > in update_display(). > > > > > > > and in the igt_fixture(), have: > > > > kmstest_set_vt_graphics_mode(); > > > > igt_install_exit_handler() > > > > > > The exit handler should be installed only after saving the original > > > value, so the handler will have the proper value to restore. Save/install > > > the exit handler only once even if force_dp_dsc_enable() is called > > > multiple times. > > > > > > > > > > > Manasi > > > > > > > > > > > > > > > > > > > > > igt_assert_f(enabled, > > > > > > - "Default DSC enable failed on Connector: %s Pipe: %s", > > > > > > + "\nDefault DSC enable failed on Connector: %s Pipe: %s", > > > > > > data->conn_name, > > > > > > kmstest_pipe_name(data->pipe)); > > > > > > } else { > > > > > > -- > > > > > > 2.19.1 > > > > > > _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [igt-dev] [PATCH i-g-t] tests/kms_dp_dsc: Force a full modeset when we force dsc enable 2019-04-03 20:49 ` Manasi Navare @ 2019-04-03 21:42 ` Imre Deak 2019-04-03 22:48 ` Manasi Navare 0 siblings, 1 reply; 19+ messages in thread From: Imre Deak @ 2019-04-03 21:42 UTC (permalink / raw) To: Manasi Navare; +Cc: igt-dev, Anusha Srivatsa, Petri Latvala On Wed, Apr 03, 2019 at 01:49:44PM -0700, Manasi Navare wrote: > [...] > Currently we have inh i915_dsc_fec_support_show: > DSC_enabled: > DSC_sink_support: > FEC_sink_support: Yes, and after the kernel change you'll have an additional Force DSC enable: yes/no line here. > And this node we open as a read only first and then as write node and > force_dsc_enable() from IGT just writes a 1 to this, so you are saying > that in the kernel _write() node, I accept this and write to > force_dsc_enable entry in this file instead of rewriting the whole > file? I'm not sure what you mean. Currently "1" or "0" is written to the debugfs file which will set intel_dp->force_dsc_enable. This doesn't need to be changed at all, no need to rewrite the whole file. > So in IGT also I will have to parse the file and write 1 for > force_dsc_enable entry? You can get the original value of it similarly to how it's done already for 'DSC_Sink_Support', by reading the file and doing strstr(buf, "Force DSC enable: yes"); on the content. > I still cant figure out how the restore() function work, we write the > stored_values and write for each of the entries in the file? And then > in the kernel debugfs_write, get these and write back to each entry? Only force_dsc_enable need to be restored, the only value that you change by writing to the debugfs file. All the rest are calculated values that you don't need to care about. --Imre _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [igt-dev] [PATCH i-g-t] tests/kms_dp_dsc: Force a full modeset when we force dsc enable 2019-04-03 21:42 ` Imre Deak @ 2019-04-03 22:48 ` Manasi Navare 2019-04-03 23:02 ` Imre Deak 0 siblings, 1 reply; 19+ messages in thread From: Manasi Navare @ 2019-04-03 22:48 UTC (permalink / raw) To: Imre Deak; +Cc: igt-dev, Anusha Srivatsa, Petri Latvala On Thu, Apr 04, 2019 at 12:42:33AM +0300, Imre Deak wrote: > On Wed, Apr 03, 2019 at 01:49:44PM -0700, Manasi Navare wrote: > > [...] > > Currently we have inh i915_dsc_fec_support_show: > > DSC_enabled: > > DSC_sink_support: > > FEC_sink_support: > > Yes, and after the kernel change you'll have an additional > > Force DSC enable: yes/no > > line here. > > > And this node we open as a read only first and then as write node and > > force_dsc_enable() from IGT just writes a 1 to this, so you are saying > > that in the kernel _write() node, I accept this and write to > > force_dsc_enable entry in this file instead of rewriting the whole > > file? > > I'm not sure what you mean. Currently "1" or "0" is written to the > debugfs file which will set intel_dp->force_dsc_enable. This doesn't > need to be changed at all, no need to rewrite the whole file. > > > So in IGT also I will have to parse the file and write 1 for > > force_dsc_enable entry? > > You can get the original value of it similarly to how it's done already > for 'DSC_Sink_Support', by reading the file and doing > > strstr(buf, "Force DSC enable: yes"); > > on the content. > > > I still cant figure out how the restore() function work, we write the > > stored_values and write for each of the entries in the file? And then > > in the kernel debugfs_write, get these and write back to each entry? > > Only force_dsc_enable need to be restored, the only value that you > change by writing to the debugfs file. All the rest are calculated > values that you don't need to care about. But now that I rething then, force_dsc_enable is always 0, it s a variable that we use to force DSC only if IGT asks to, so for test we force it to 1 and then after the test always clear it since the original value is always 0. So why do we need a separate restore() for just force_dsc_enable if the value is always 0. Manasi > > --Imre _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [igt-dev] [PATCH i-g-t] tests/kms_dp_dsc: Force a full modeset when we force dsc enable 2019-04-03 22:48 ` Manasi Navare @ 2019-04-03 23:02 ` Imre Deak 2019-04-03 23:11 ` Manasi Navare 0 siblings, 1 reply; 19+ messages in thread From: Imre Deak @ 2019-04-03 23:02 UTC (permalink / raw) To: Manasi Navare; +Cc: igt-dev, Anusha Srivatsa, Petri Latvala On Wed, Apr 03, 2019 at 03:48:24PM -0700, Manasi Navare wrote: > On Thu, Apr 04, 2019 at 12:42:33AM +0300, Imre Deak wrote: > > On Wed, Apr 03, 2019 at 01:49:44PM -0700, Manasi Navare wrote: > > > [...] > > > Currently we have inh i915_dsc_fec_support_show: > > > DSC_enabled: > > > DSC_sink_support: > > > FEC_sink_support: > > > > Yes, and after the kernel change you'll have an additional > > > > Force DSC enable: yes/no > > > > line here. > > > > > And this node we open as a read only first and then as write node and > > > force_dsc_enable() from IGT just writes a 1 to this, so you are saying > > > that in the kernel _write() node, I accept this and write to > > > force_dsc_enable entry in this file instead of rewriting the whole > > > file? > > > > I'm not sure what you mean. Currently "1" or "0" is written to the > > debugfs file which will set intel_dp->force_dsc_enable. This doesn't > > need to be changed at all, no need to rewrite the whole file. > > > > > So in IGT also I will have to parse the file and write 1 for > > > force_dsc_enable entry? > > > > You can get the original value of it similarly to how it's done already > > for 'DSC_Sink_Support', by reading the file and doing > > > > strstr(buf, "Force DSC enable: yes"); > > > > on the content. > > > > > I still cant figure out how the restore() function work, we write the > > > stored_values and write for each of the entries in the file? And then > > > in the kernel debugfs_write, get these and write back to each entry? > > > > Only force_dsc_enable need to be restored, the only value that you > > change by writing to the debugfs file. All the rest are calculated > > values that you don't need to care about. > > But now that I rething then, force_dsc_enable is always 0, it s a variable > that we use to force DSC only if IGT asks to, so for test we force it to 1 > and then after the test always clear it since the original value is always 0. > So why do we need a separate restore() for just force_dsc_enable if the value > is always 0. It's not necessarily 0. Before the test starts someone could've written 1 to that file. > > Manasi > > > > > --Imre _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [igt-dev] [PATCH i-g-t] tests/kms_dp_dsc: Force a full modeset when we force dsc enable 2019-04-03 23:02 ` Imre Deak @ 2019-04-03 23:11 ` Manasi Navare 2019-04-03 23:15 ` Imre Deak 0 siblings, 1 reply; 19+ messages in thread From: Manasi Navare @ 2019-04-03 23:11 UTC (permalink / raw) To: Imre Deak; +Cc: igt-dev, Anusha Srivatsa, Petri Latvala On Thu, Apr 04, 2019 at 02:02:42AM +0300, Imre Deak wrote: > On Wed, Apr 03, 2019 at 03:48:24PM -0700, Manasi Navare wrote: > > On Thu, Apr 04, 2019 at 12:42:33AM +0300, Imre Deak wrote: > > > On Wed, Apr 03, 2019 at 01:49:44PM -0700, Manasi Navare wrote: > > > > [...] > > > > Currently we have inh i915_dsc_fec_support_show: > > > > DSC_enabled: > > > > DSC_sink_support: > > > > FEC_sink_support: > > > > > > Yes, and after the kernel change you'll have an additional > > > > > > Force DSC enable: yes/no > > > > > > line here. > > > > > > > And this node we open as a read only first and then as write node and > > > > force_dsc_enable() from IGT just writes a 1 to this, so you are saying > > > > that in the kernel _write() node, I accept this and write to > > > > force_dsc_enable entry in this file instead of rewriting the whole > > > > file? > > > > > > I'm not sure what you mean. Currently "1" or "0" is written to the > > > debugfs file which will set intel_dp->force_dsc_enable. This doesn't > > > need to be changed at all, no need to rewrite the whole file. > > > > > > > So in IGT also I will have to parse the file and write 1 for > > > > force_dsc_enable entry? > > > > > > You can get the original value of it similarly to how it's done already > > > for 'DSC_Sink_Support', by reading the file and doing > > > > > > strstr(buf, "Force DSC enable: yes"); > > > > > > on the content. > > > > > > > I still cant figure out how the restore() function work, we write the > > > > stored_values and write for each of the entries in the file? And then > > > > in the kernel debugfs_write, get these and write back to each entry? > > > > > > Only force_dsc_enable need to be restored, the only value that you > > > change by writing to the debugfs file. All the rest are calculated > > > values that you don't need to care about. > > > > But now that I rething then, force_dsc_enable is always 0, it s a variable > > that we use to force DSC only if IGT asks to, so for test we force it to 1 > > and then after the test always clear it since the original value is always 0. > > So why do we need a separate restore() for just force_dsc_enable if the value > > is always 0. > > It's not necessarily 0. Before the test starts someone could've written > 1 to that file. Hmm, so now the use of this force_dsc_enable in kernel is only for this specific IGT and the kernel doesnt touch this. But because in the future the kernel might set it to true, we want to make sure that the IGT doesnt change the kernel behaviour after the test is run, is my understanding correct? So basically adding that now for future proofing it? Also on the igt_install_exit_handler(), should the test_cleanup be now part of this handler? Manasi > > > > > Manasi > > > > > > > > --Imre _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [igt-dev] [PATCH i-g-t] tests/kms_dp_dsc: Force a full modeset when we force dsc enable 2019-04-03 23:11 ` Manasi Navare @ 2019-04-03 23:15 ` Imre Deak 2019-04-04 23:44 ` Manasi Navare 0 siblings, 1 reply; 19+ messages in thread From: Imre Deak @ 2019-04-03 23:15 UTC (permalink / raw) To: Manasi Navare; +Cc: igt-dev, Anusha Srivatsa, Petri Latvala On Wed, Apr 03, 2019 at 04:11:49PM -0700, Manasi Navare wrote: > On Thu, Apr 04, 2019 at 02:02:42AM +0300, Imre Deak wrote: > > On Wed, Apr 03, 2019 at 03:48:24PM -0700, Manasi Navare wrote: > > > On Thu, Apr 04, 2019 at 12:42:33AM +0300, Imre Deak wrote: > > > > On Wed, Apr 03, 2019 at 01:49:44PM -0700, Manasi Navare wrote: > > > > > [...] > > > > > Currently we have inh i915_dsc_fec_support_show: > > > > > DSC_enabled: > > > > > DSC_sink_support: > > > > > FEC_sink_support: > > > > > > > > Yes, and after the kernel change you'll have an additional > > > > > > > > Force DSC enable: yes/no > > > > > > > > line here. > > > > > > > > > And this node we open as a read only first and then as write node and > > > > > force_dsc_enable() from IGT just writes a 1 to this, so you are saying > > > > > that in the kernel _write() node, I accept this and write to > > > > > force_dsc_enable entry in this file instead of rewriting the whole > > > > > file? > > > > > > > > I'm not sure what you mean. Currently "1" or "0" is written to the > > > > debugfs file which will set intel_dp->force_dsc_enable. This doesn't > > > > need to be changed at all, no need to rewrite the whole file. > > > > > > > > > So in IGT also I will have to parse the file and write 1 for > > > > > force_dsc_enable entry? > > > > > > > > You can get the original value of it similarly to how it's done already > > > > for 'DSC_Sink_Support', by reading the file and doing > > > > > > > > strstr(buf, "Force DSC enable: yes"); > > > > > > > > on the content. > > > > > > > > > I still cant figure out how the restore() function work, we write the > > > > > stored_values and write for each of the entries in the file? And then > > > > > in the kernel debugfs_write, get these and write back to each entry? > > > > > > > > Only force_dsc_enable need to be restored, the only value that you > > > > change by writing to the debugfs file. All the rest are calculated > > > > values that you don't need to care about. > > > > > > But now that I rething then, force_dsc_enable is always 0, it s a variable > > > that we use to force DSC only if IGT asks to, so for test we force it to 1 > > > and then after the test always clear it since the original value is always 0. > > > So why do we need a separate restore() for just force_dsc_enable if the value > > > is always 0. > > > > It's not necessarily 0. Before the test starts someone could've written > > 1 to that file. > > Hmm, so now the use of this force_dsc_enable in kernel is only for > this specific IGT and the kernel doesnt touch this. But because in the > future the kernel might set it to true, we want to make sure that the > IGT doesnt change the kernel behaviour after the test is run, is my > understanding correct? So basically adding that now for future > proofing it? No, anybody running tests manually could set it and expect it to be preserved. > Also on the igt_install_exit_handler(), should the test_cleanup be now > part of this handler? No need for that, any required modeset specific cleanup will be handled automatically. > Manasi > > > > > > > > > Manasi > > > > > > > > > > > --Imre _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [igt-dev] [PATCH i-g-t] tests/kms_dp_dsc: Force a full modeset when we force dsc enable 2019-04-03 23:15 ` Imre Deak @ 2019-04-04 23:44 ` Manasi Navare 2019-04-05 10:06 ` Imre Deak 0 siblings, 1 reply; 19+ messages in thread From: Manasi Navare @ 2019-04-04 23:44 UTC (permalink / raw) To: Imre Deak; +Cc: igt-dev, Anusha Srivatsa, Petri Latvala On Thu, Apr 04, 2019 at 02:15:06AM +0300, Imre Deak wrote: > On Wed, Apr 03, 2019 at 04:11:49PM -0700, Manasi Navare wrote: > > On Thu, Apr 04, 2019 at 02:02:42AM +0300, Imre Deak wrote: > > > On Wed, Apr 03, 2019 at 03:48:24PM -0700, Manasi Navare wrote: > > > > On Thu, Apr 04, 2019 at 12:42:33AM +0300, Imre Deak wrote: > > > > > On Wed, Apr 03, 2019 at 01:49:44PM -0700, Manasi Navare wrote: > > > > > > [...] > > > > > > Currently we have inh i915_dsc_fec_support_show: > > > > > > DSC_enabled: > > > > > > DSC_sink_support: > > > > > > FEC_sink_support: > > > > > > > > > > Yes, and after the kernel change you'll have an additional > > > > > > > > > > Force DSC enable: yes/no > > > > > > > > > > line here. > > > > > > > > > > > And this node we open as a read only first and then as write node and > > > > > > force_dsc_enable() from IGT just writes a 1 to this, so you are saying > > > > > > that in the kernel _write() node, I accept this and write to > > > > > > force_dsc_enable entry in this file instead of rewriting the whole > > > > > > file? > > > > > > > > > > I'm not sure what you mean. Currently "1" or "0" is written to the > > > > > debugfs file which will set intel_dp->force_dsc_enable. This doesn't > > > > > need to be changed at all, no need to rewrite the whole file. > > > > > > > > > > > So in IGT also I will have to parse the file and write 1 for > > > > > > force_dsc_enable entry? > > > > > > > > > > You can get the original value of it similarly to how it's done already > > > > > for 'DSC_Sink_Support', by reading the file and doing > > > > > > > > > > strstr(buf, "Force DSC enable: yes"); > > > > > > > > > > on the content. > > > > > > > > > > > I still cant figure out how the restore() function work, we write the > > > > > > stored_values and write for each of the entries in the file? And then > > > > > > in the kernel debugfs_write, get these and write back to each entry? > > > > > > > > > > Only force_dsc_enable need to be restored, the only value that you > > > > > change by writing to the debugfs file. All the rest are calculated > > > > > values that you don't need to care about. > > > > > > > > But now that I rething then, force_dsc_enable is always 0, it s a variable > > > > that we use to force DSC only if IGT asks to, so for test we force it to 1 > > > > and then after the test always clear it since the original value is always 0. > > > > So why do we need a separate restore() for just force_dsc_enable if the value > > > > is always 0. > > > > > > It's not necessarily 0. Before the test starts someone could've written > > > 1 to that file. > > > > Hmm, so now the use of this force_dsc_enable in kernel is only for > > this specific IGT and the kernel doesnt touch this. But because in the > > future the kernel might set it to true, we want to make sure that the > > IGT doesnt change the kernel behaviour after the test is run, is my > > understanding correct? So basically adding that now for future > > proofing it? > > No, anybody running tests manually could set it and expect it to be > preserved. > > > Also on the igt_install_exit_handler(), should the test_cleanup be now > > part of this handler? > > No need for that, any required modeset specific cleanup will be handled > automatically. So just call igt_install_exit_handler() without pasisng any function pointer to do just the default cleanups? Manasi > > > Manasi > > > > > > > > > > > > > Manasi > > > > > > > > > > > > > > --Imre _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [igt-dev] [PATCH i-g-t] tests/kms_dp_dsc: Force a full modeset when we force dsc enable 2019-04-04 23:44 ` Manasi Navare @ 2019-04-05 10:06 ` Imre Deak 2019-04-05 18:28 ` Manasi Navare 0 siblings, 1 reply; 19+ messages in thread From: Imre Deak @ 2019-04-05 10:06 UTC (permalink / raw) To: Manasi Navare; +Cc: igt-dev, Anusha Srivatsa, Petri Latvala On Thu, Apr 04, 2019 at 04:44:35PM -0700, Manasi Navare wrote: > [...] > So just call igt_install_exit_handler() without pasisng any function pointer to do > just the default cleanups? You need to pass it a pointer to a new function you add. The new function should write the original value to the debugfs file. --Imre _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [igt-dev] [PATCH i-g-t] tests/kms_dp_dsc: Force a full modeset when we force dsc enable 2019-04-05 10:06 ` Imre Deak @ 2019-04-05 18:28 ` Manasi Navare 0 siblings, 0 replies; 19+ messages in thread From: Manasi Navare @ 2019-04-05 18:28 UTC (permalink / raw) To: Imre Deak; +Cc: igt-dev, Anusha Srivatsa, Petri Latvala On Fri, Apr 05, 2019 at 01:06:46PM +0300, Imre Deak wrote: > On Thu, Apr 04, 2019 at 04:44:35PM -0700, Manasi Navare wrote: > > [...] > > So just call igt_install_exit_handler() without pasisng any function pointer to do > > just the default cleanups? > > You need to pass it a pointer to a new function you add. The new > function should write the original value to the debugfs file. Ok, I had added the restore_value() function to the test_cleanup that I call after update_display() but yes adding that to the exit handler function would be a good idea. the test cleanup function that I have now does: if (data->output) { igt_plane_set_fb(primary, NULL); igt_display_commit(&data->display); igt_remove_fb(data->drm_fd, &data->fb_test_pattern); } Hmm, I can still keep this after each test as is correct? Manasi > > --Imre _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [igt-dev] [PATCH i-g-t] tests/kms_dp_dsc: Force a full modeset when we force dsc enable 2019-04-02 19:31 [igt-dev] [PATCH i-g-t] tests/kms_dp_dsc: Force a full modeset when we force dsc enable Manasi Navare ` (3 preceding siblings ...) 2019-04-03 13:25 ` Imre Deak @ 2019-04-03 17:38 ` Srivatsa, Anusha 4 siblings, 0 replies; 19+ messages in thread From: Srivatsa, Anusha @ 2019-04-03 17:38 UTC (permalink / raw) To: Navare, Manasi D, igt-dev; +Cc: Latvala, Petri I tried the below changes on ICL system. The changes below as they are is also failing the test. Anusha >-----Original Message----- >From: Navare, Manasi D >Sent: Tuesday, April 2, 2019 12:31 PM >To: igt-dev@lists.freedesktop.org >Cc: Navare, Manasi D <manasi.d.navare@intel.com>; Latvala, Petri ><petri.latvala@intel.com>; Srivatsa, Anusha <anusha.srivatsa@intel.com>; Deak, >Imre <imre.deak@intel.com> >Subject: [PATCH i-g-t] tests/kms_dp_dsc: Force a full modeset when we force dsc >enable > >DSC enable gets configured during compute_config and needs a full modeset to >force DSC. >Sometimes in between the tests, if the initial output is same as the mode being >set for DSC then it will not do a full modeset. >So we disable the output before forcing a mode with DSC enable. > >Fixes: db19bccc1c22 ("test/kms_dp_dsc: Basic KMS test to validate VESA DSC on >DP/eDP") >Cc: Petri Latvala <petri.latvala@intel.com> >Cc: Anusha Srivatsa <anusha.srivatsa@intel.com> >Cc: Imre Deak <imre.deak@intel.com> >Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> >--- > tests/kms_dp_dsc.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > >diff --git a/tests/kms_dp_dsc.c b/tests/kms_dp_dsc.c index da93cd74..f3a61029 >100644 >--- a/tests/kms_dp_dsc.c >+++ b/tests/kms_dp_dsc.c >@@ -174,6 +174,11 @@ static void update_display(data_t *data, enum >dsc_test_type test_type) > &data->fb_test_pattern); > primary = igt_output_get_plane_type(data->output, > >DRM_PLANE_TYPE_PRIMARY); >+ /* Disable the output first */ >+ igt_plane_set_fb(primary, NULL); >+ igt_output_set_pipe(data->output, PIPE_NONE); >+ >+ /* Now set the output to the desired mode */ > igt_plane_set_fb(primary, &data->fb_test_pattern); > igt_display_commit(&data->display); > >@@ -187,7 +192,7 @@ static void update_display(data_t *data, enum >dsc_test_type test_type) > clear_dp_dsc_enable(data); > > igt_assert_f(enabled, >- "Default DSC enable failed on Connector: %s Pipe: >%s", >+ "\nDefault DSC enable failed on Connector: %s Pipe: >%s", > data->conn_name, > kmstest_pipe_name(data->pipe)); > } else { >-- >2.19.1 _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2019-04-05 18:26 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-04-02 19:31 [igt-dev] [PATCH i-g-t] tests/kms_dp_dsc: Force a full modeset when we force dsc enable Manasi Navare 2019-04-02 20:10 ` [igt-dev] ✓ Fi.CI.BAT: success for " Patchwork 2019-04-03 7:46 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork 2019-04-03 10:06 ` [igt-dev] [PATCH i-g-t] " Petri Latvala 2019-04-03 13:25 ` Imre Deak 2019-04-03 18:20 ` Manasi Navare 2019-04-03 19:33 ` Imre Deak 2019-04-03 20:17 ` Manasi Navare 2019-04-03 20:34 ` Imre Deak 2019-04-03 20:49 ` Manasi Navare 2019-04-03 21:42 ` Imre Deak 2019-04-03 22:48 ` Manasi Navare 2019-04-03 23:02 ` Imre Deak 2019-04-03 23:11 ` Manasi Navare 2019-04-03 23:15 ` Imre Deak 2019-04-04 23:44 ` Manasi Navare 2019-04-05 10:06 ` Imre Deak 2019-04-05 18:28 ` Manasi Navare 2019-04-03 17:38 ` Srivatsa, Anusha
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.