| Message ID | 20240522220849.33343-4-mario.limonciello@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add support for testing power saving policy DRM property | expand |
Thanks for the tests! FYI IGT patches should also cc igt-dev@lists.freedesktop.org Some comments inline: On 2024-05-22 18:08, Mario Limonciello wrote: > From: Mario Limonciello <superm1@ubuntu.com> > > When the "panel power saving" property is set to forbidden the > compositor has indicated that userspace prefers to have color > accuracy and fidelity instead of power saving. > > Verify that the sysfs file behaves as expected in this situation. > > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> > --- > tests/amdgpu/amd_abm.c | 39 +++++++++++++++++++++++++++++++++++++++ > 1 file changed, 39 insertions(+) > > diff --git a/tests/amdgpu/amd_abm.c b/tests/amdgpu/amd_abm.c > index f74c3012c..3fa1366fa 100644 > --- a/tests/amdgpu/amd_abm.c > +++ b/tests/amdgpu/amd_abm.c > @@ -365,6 +365,43 @@ static void abm_gradual(data_t *data) > } > } > > + > +static void abm_forbidden(data_t *data) > +{ > + igt_output_t *output; > + enum pipe pipe; > + int target, r; > + > + for_each_pipe_with_valid_output(&data->display, pipe, output) { > + if (output->config.connector->connector_type != DRM_MODE_CONNECTOR_eDP) > + continue; > + > + r = clear_power_saving_policy(data->drm_fd, output); > + if (r == -ENODEV) { > + igt_skip("No power saving policy prop\n"); > + return; > + } > + igt_assert_eq(r, 0); > + > + target = 3; > + r = set_abm_level(data, output, target); > + igt_assert_eq(r, 0); > + > + r = set_panel_power_saving_policy(data->drm_fd, output, DRM_MODE_REQUIRE_COLOR_ACCURACY); > + igt_assert_eq(r, 0); > + > + target = 0; Is there a purpose of setting target abm to 0 (disabled) here? I suppose it should fail given that we've set REQUIRE_COLOR_ACCURACY. Though I'm not sure why we can't keep target = 3. Thanks, Leo > + r = set_abm_level(data, output, target); > + igt_assert_eq(r, -1); > + > + r = clear_power_saving_policy(data->drm_fd, output); > + igt_assert_eq(r, 0); > + > + r = set_abm_level(data, output, target); > + igt_assert_eq(r, 0); > + } > +} > + > igt_main > { > data_t data = {}; > @@ -393,6 +430,8 @@ igt_main > abm_enabled(&data); > igt_subtest("abm_gradual") > abm_gradual(&data); > + igt_subtest("abm_forbidden") > + abm_forbidden(&data); > > igt_fixture { > igt_display_fini(&data.display);
On 6/18/2024 15:20, Leo Li wrote: > > Thanks for the tests! FYI IGT patches should also cc > igt-dev@lists.freedesktop.org > > Some comments inline: > > On 2024-05-22 18:08, Mario Limonciello wrote: >> From: Mario Limonciello <superm1@ubuntu.com> >> >> When the "panel power saving" property is set to forbidden the >> compositor has indicated that userspace prefers to have color >> accuracy and fidelity instead of power saving. >> >> Verify that the sysfs file behaves as expected in this situation. >> >> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> >> --- >> tests/amdgpu/amd_abm.c | 39 +++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 39 insertions(+) >> >> diff --git a/tests/amdgpu/amd_abm.c b/tests/amdgpu/amd_abm.c >> index f74c3012c..3fa1366fa 100644 >> --- a/tests/amdgpu/amd_abm.c >> +++ b/tests/amdgpu/amd_abm.c >> @@ -365,6 +365,43 @@ static void abm_gradual(data_t *data) >> } >> } >> + >> +static void abm_forbidden(data_t *data) >> +{ >> + igt_output_t *output; >> + enum pipe pipe; >> + int target, r; >> + >> + for_each_pipe_with_valid_output(&data->display, pipe, output) { >> + if (output->config.connector->connector_type != >> DRM_MODE_CONNECTOR_eDP) >> + continue; >> + >> + r = clear_power_saving_policy(data->drm_fd, output); >> + if (r == -ENODEV) { >> + igt_skip("No power saving policy prop\n"); >> + return; >> + } >> + igt_assert_eq(r, 0); >> + >> + target = 3; >> + r = set_abm_level(data, output, target); >> + igt_assert_eq(r, 0); >> + >> + r = set_panel_power_saving_policy(data->drm_fd, output, >> DRM_MODE_REQUIRE_COLOR_ACCURACY); >> + igt_assert_eq(r, 0); >> + >> + target = 0; > > Is there a purpose of setting target abm to 0 (disabled) here? > > I suppose it should fail given that we've set REQUIRE_COLOR_ACCURACY. > Though I'm > not sure why we can't keep target = 3. Yes I think this would work as well to prove a failure. I'll change it. > > Thanks, > Leo > >> + r = set_abm_level(data, output, target); >> + igt_assert_eq(r, -1); >> + >> + r = clear_power_saving_policy(data->drm_fd, output); >> + igt_assert_eq(r, 0); >> + >> + r = set_abm_level(data, output, target); >> + igt_assert_eq(r, 0); >> + } >> +} >> + >> igt_main >> { >> data_t data = {}; >> @@ -393,6 +430,8 @@ igt_main >> abm_enabled(&data); >> igt_subtest("abm_gradual") >> abm_gradual(&data); >> + igt_subtest("abm_forbidden") >> + abm_forbidden(&data); >> igt_fixture { >> igt_display_fini(&data.display);
diff --git a/tests/amdgpu/amd_abm.c b/tests/amdgpu/amd_abm.c index f74c3012c..3fa1366fa 100644 --- a/tests/amdgpu/amd_abm.c +++ b/tests/amdgpu/amd_abm.c @@ -365,6 +365,43 @@ static void abm_gradual(data_t *data) } } + +static void abm_forbidden(data_t *data) +{ + igt_output_t *output; + enum pipe pipe; + int target, r; + + for_each_pipe_with_valid_output(&data->display, pipe, output) { + if (output->config.connector->connector_type != DRM_MODE_CONNECTOR_eDP) + continue; + + r = clear_power_saving_policy(data->drm_fd, output); + if (r == -ENODEV) { + igt_skip("No power saving policy prop\n"); + return; + } + igt_assert_eq(r, 0); + + target = 3; + r = set_abm_level(data, output, target); + igt_assert_eq(r, 0); + + r = set_panel_power_saving_policy(data->drm_fd, output, DRM_MODE_REQUIRE_COLOR_ACCURACY); + igt_assert_eq(r, 0); + + target = 0; + r = set_abm_level(data, output, target); + igt_assert_eq(r, -1); + + r = clear_power_saving_policy(data->drm_fd, output); + igt_assert_eq(r, 0); + + r = set_abm_level(data, output, target); + igt_assert_eq(r, 0); + } +} + igt_main { data_t data = {}; @@ -393,6 +430,8 @@ igt_main abm_enabled(&data); igt_subtest("abm_gradual") abm_gradual(&data); + igt_subtest("abm_forbidden") + abm_forbidden(&data); igt_fixture { igt_display_fini(&data.display);