[igt-dev] [i-g-t 3/5] tests/kms_vrr: Add Negative tests to validate VRR
Modem, Bhanuprakash
bhanuprakash.modem at intel.com
Tue Mar 8 04:13:50 UTC 2022
On Tue-08-03-2022 01:36 am, Navare, Manasi wrote:
> On Fri, Mar 04, 2022 at 09:32:29AM +0530, Modem, Bhanuprakash wrote:
>> On Fri-04-03-2022 06:50 am, Navare, Manasi wrote:
>>> 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 <manasi.d.navare at intel.com>
>>>> Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem at intel.com>
>>>> ---
>>>> 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.
>>
>> Hmm, intel_vrr_compute_config() will just return & even don't care about
>> "vrr_enabled" if the panel is Non-VRR.
>>
>> In general, if we try to commit with invalid config, Kernel must reject
>> (with -22 invalid arg) right? Here, we requested to enable VRR, but Kernel
>> returned success without enabling it.
>>
>> So the flow would be:
>> * Set "vrr_enabled" on selected pipe & do modeset
>> * Readback the "vrr_enabled" prop to make sure that Kernel enabled the VRR
>> on selected pipe.
>>
>> Am I missing anything?
>
> Yes so in case of VRR, its an enhancement to improve the visual quality.
> But if userspace requests VRR but the panel doesnt support it, the kernel will
> not reject the mode since having some screen output without VRR is better than no output with a black screen.
Got it.
>
> So it will succeed with the modeset.
> However like the VRR enable register will not be set for example, so if we absolutely want to add a negative test
> we can probe the VRR enable register through debugfs that reads the VRR enable i915 register which in this case
> will just be 0.
Instead of probing the vrr registers, I think driver should update
"VRR_ENABLED" property with 0. Also, community is not recommending to
add new debugfs.
I tried to readback "VRR_ENABLED" prop, but Kernel is not clearing this
prop if panel is non-VRR.
https://intel-gfx-ci.01.org/tree/drm-tip/TrybotIGT_488/fi-tgl-1115g4/igt@kms_vrr@negative-basic@pipe-a-dp-1.html
- Bhanu
>
> Manasi
>
>>
>> - Bhanu
>>
>>>
>>> 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
>>>>
>>
More information about the igt-dev
mailing list