From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by gabe.freedesktop.org (Postfix) with ESMTPS id E2B9B10E40E for ; Fri, 4 Mar 2022 01:19:39 +0000 (UTC) Date: Thu, 3 Mar 2022 17:20:29 -0800 From: "Navare, Manasi" To: Bhanuprakash Modem Message-ID: <20220304012021.GB5029@labuser-Z97X-UD5H> References: <20220224051648.91470-1-bhanuprakash.modem@intel.com> <20220224051648.91470-4-bhanuprakash.modem@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220224051648.91470-4-bhanuprakash.modem@intel.com> Subject: Re: [igt-dev] [i-g-t 3/5] tests/kms_vrr: Add Negative tests to validate VRR List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: igt-dev@lists.freedesktop.org Errors-To: igt-dev-bounces@lists.freedesktop.org Sender: "igt-dev" List-ID: On Thu, Feb 24, 2022 at 10:46:46AM +0530, Bhanuprakash Modem wrote: > This patch will try to enable VRR on Non-VRR panel. VRR should > not be enabled on the Non-VRR panel. Hence Kernel should reject > the commit. > > Cc: Manasi Navare > Signed-off-by: Bhanuprakash Modem > --- > tests/kms_vrr.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 67 insertions(+), 2 deletions(-) > > diff --git a/tests/kms_vrr.c b/tests/kms_vrr.c > index 8976d4a6c3..7deab77311 100644 > --- a/tests/kms_vrr.c > +++ b/tests/kms_vrr.c > @@ -45,6 +45,7 @@ enum { > TEST_DPMS = 1 << 0, > TEST_SUSPEND = 1 << 1, > TEST_FLIPLINE = 1 << 2, > + TEST_NEGATIVE = 1 << 3, > }; > > typedef struct range { > @@ -244,6 +245,34 @@ static void prepare_test(data_t *data, igt_output_t *output, enum pipe pipe) > igt_display_commit2(&data->display, COMMIT_ATOMIC); > } > > +static void prepare_negative_test(data_t *data, igt_output_t *output, enum pipe pipe) > +{ > + drmModeModeInfo *mode; > + > + /* Reset output */ > + igt_display_reset(&data->display); > + igt_output_set_pipe(output, pipe); > + > + /* Capture VRR range */ > + data->range = get_vrr_range(data, output); > + > + /* Prepare resources */ > + mode = igt_output_get_mode(output); > + igt_assert(igt_create_fb(data->drm_fd, mode->hdisplay, mode->vdisplay, > + DRM_FORMAT_XRGB8888, DRM_FORMAT_MOD_LINEAR, > + &data->fb0)); > + > + /* Take care of any required modesetting before the test begins. */ > + data->primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY); > + igt_plane_set_fb(data->primary, &data->fb0); > + > + /* Clear vrr_enabled state before enabling it, because > + * it might be left enabled if the previous test fails. > + */ > + igt_pipe_set_prop_value(&data->display, pipe, IGT_CRTC_VRR_ENABLED, 0); > + igt_display_commit2(&data->display, COMMIT_ATOMIC); > +} > + > /* Performs an atomic non-blocking page-flip on a pipe. */ > static void > do_flip(data_t *data, igt_fb_t *fb) > @@ -430,6 +459,32 @@ test_basic(data_t *data, enum pipe pipe, igt_output_t *output, uint32_t flags) > igt_remove_fb(data->drm_fd, &data->fb0); > } > > +/* VRR on Non-VRR panel: VRR should not be enabled on the Non-VRR panel. > + * Kernel should reject the commit. > + */ > +static void > +test_negative_basic(data_t *data, enum pipe pipe, > + igt_output_t *output, uint32_t flags) > +{ > + int ret; > + > + igt_info("VRR Negative Test execution on %s, PIPE_%s.\n", > + output->name, kmstest_pipe_name(pipe)); > + > + prepare_negative_test(data, output, pipe); > + igt_pipe_set_prop_value(&data->display, pipe, IGT_CRTC_VRR_ENABLED, 1); > + > + ret = igt_display_try_commit2(&data->display, COMMIT_ATOMIC); > + igt_assert_f(ret == 0, "VRR shouln't be enabled on Non-VRR panel.\n"); No this is not correct since in the kernel, if its a non VRR panel then it will just do the modeset without enabling VRR so you will just see the visual artifacts but not reject it. Manasi > + > + /* Clean-up */ > + igt_plane_set_fb(data->primary, NULL); > + igt_output_set_pipe(output, PIPE_NONE); > + set_vrr_on_pipe(data, pipe, false); > + > + igt_remove_fb(data->drm_fd, &data->fb0); > +} > + > /* Runs tests on outputs that are VRR capable. */ > static void > run_vrr_test(data_t *data, test_t test, uint32_t flags) > @@ -439,8 +494,14 @@ run_vrr_test(data_t *data, test_t test, uint32_t flags) > for_each_connected_output(&data->display, output) { > enum pipe pipe; > > - if (!has_vrr(output)) > - continue; > + /* For Negative tests, panel should be non-vrr. */ > + if (flags & TEST_NEGATIVE) { > + if (has_vrr(output)) > + continue; > + } else { > + if (!has_vrr(output)) > + continue; > + } > > for_each_pipe(&data->display, pipe) { > if (igt_pipe_connector_valid(pipe, output)) { > @@ -486,6 +547,10 @@ igt_main > igt_subtest_with_dynamic("flipline") > run_vrr_test(&data, test_basic, TEST_FLIPLINE); > > + igt_describe("Make sure that VRR should not be enabled on the Non-VRR panel."); > + igt_subtest_with_dynamic("negative-basic") > + run_vrr_test(&data, test_negative_basic, TEST_NEGATIVE); > + > igt_fixture { > igt_display_fini(&data.display); > } > -- > 2.35.0 >