Re: [PATCH v3 1/2] tests/kms_vrr: Add a subtest for seamless modeset to a virtual LRR mode

["Modem, Bhanuprakash" <bhanuprakash.modem@intel.com>]

Hi Manasi,

Sorry for replying late, I was in vacation.

On 27-04-2024 12:54 am, Manasi Navare wrote:
> Hi Bhanu, Jani,
> 
> I do see that even the other vrr tests : seamless-rr-switch-vrr /drrs/
> basic-fastset are getting skipped on BAT, so its not related to this
> test. I am able to run it here locally and test.
> Could you please take a look and see why all of these would be
> skipped? Seems unrelated to this change.

In case of seamless-rr-switch-(vrr|drrs), these skips are expected, as 
we don't have any valid config. These tests are eDP specific & panel 
should have the modes with different refresh rates & clocks with same 
resolution.

In case of seamless-rr-switch-virtual, it is a connector agnostic test, 
it should run on one of the BAT machines.

Can you please review the helper low_rr_mode_with_same_res() once, as 
you are using this one to pick the low RR mode?

> 
> Regards
> Manasi
> 
> On Fri, Apr 26, 2024 at 12:48 AM Saarinen, Jani <jani.saarinen@intel.com> wrote:
>>
>> Hi,
>>> -----Original Message-----
>>> From: igt-dev <igt-dev-bounces@lists.freedesktop.org> On Behalf Of Modem,
>>> Bhanuprakash
>>> Sent: Friday, 26 April 2024 6.44
>>> To: Manasi Navare <navaremanasi@chromium.org>
>>> Cc: igt-dev@lists.freedesktop.org; Drew Davenport
>>> <ddavenport@chromium.org>; Ville Syrjälä <ville.syrjala@linux.intel.com>;
>>> Sean Paul <seanpaul@chromium.org>
>>> Subject: Re: [PATCH v3 1/2] tests/kms_vrr: Add a subtest for seamless
>>> modeset to a virtual LRR mode
>>>
>>> Hi Manasi,
>>>
>>> On 25-04-2024 08:50 pm, Manasi Navare wrote:
>>>> Hi Bhanu,
>>>>
>>>> Thanks for the review.
>>>> This is not specific to edp, it applies to all connectors.
>>>>
>>>> If the BAT results look okay and I have your r-b, could we get this merged?
>> Did you look those.
>>
>>>
>>> Looks, BAT results are not OK to me.
>> Right. To me looks so too.
>>
>>>
>>> The newly added test is getting skip on VRR configs:
>>> https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_11065/bat-
>>> all.html?testfilter=kms_vrr&hosts=bat-adlp-6%7Cbat-adlp-9%7Cbat-rpls-
>>> 4%7Cbat-adls-6
>>>
>>> - Bhanu
>>>
>>>>
>>>> Regards
>>>> Manasi
>>>>
>>>> On Thu, Apr 25, 2024 at 12:39 AM Modem, Bhanuprakash
>>>> <bhanuprakash.modem@intel.com> wrote:
>>>>>
>>>>> Hi Manasi,
>>>>>
>>>>> On 25-04-2024 01:43 am, Manasi Navare wrote:
>>>>>> This subtest validates LRR fastset functionality by testing seamless
>>>>>> switching without full modeset to any of the virtual LRR mode within VRR
>>> range.
>>>>>> Here we start from highest refresh rate mode and then switch to
>>>>>> virtual LRR modes in the steps of 10Hz within the VRR range.
>>>>>>
>>>>>> This is used to test the LRR fastset functionality of the driver.
>>>>>>
>>>>>> v5:
>>>>>> - Pass a reference to flip_and_measure (bhanu)
>>>>>> - Add a new line between between declaration and code logic
>>>>>>
>>>>>> v4:
>>>>>> - Change the test name to align with drrs/vrr tests (Bhanu)
>>>>>> - Fix some build warnings due to rebase
>>>>>> - Use a local virtual_mode variable
>>>>>>
>>>>>> v3:
>>>>>> - Fix build error due to rebase (Manasi)
>>>>>>
>>>>>> Cc: Drew Davenport <ddavenport@chromium.org>
>>>>>> Cc: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
>>>>>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>>>> Cc: Sean Paul <seanpaul@chromium.org>
>>>>>> Signed-off-by: Manasi Navare <navaremanasi@chromium.org>
>>>>>> ---
>>>>>>     tests/kms_vrr.c | 73
>>> ++++++++++++++++++++++++++++++++++++++++++++++---
>>>>>>     1 file changed, 69 insertions(+), 4 deletions(-)
>>>>>>
>>>>>> diff --git a/tests/kms_vrr.c b/tests/kms_vrr.c index
>>>>>> 15c62b34b..7f64d6806 100644
>>>>>> --- a/tests/kms_vrr.c
>>>>>> +++ b/tests/kms_vrr.c
>>>>>> @@ -66,6 +66,11 @@
>>>>>>      * Description: Test to switch RR seamlessly without modeset.
>>>>>>      * Functionality: adaptive_sync, drrs
>>>>>>      *
>>>>>> + * SUBTEST: seamless-rr-switch-virtual
>>>>>> + * Description: Test to create a Virtual Mode in VRR range and switch to
>>> it
>>>>>> + *           without a full modeset.
>>>>>> + * Functionality: LRR
>>>>>> + *
>>>>>>      * SUBTEST: max-min
>>>>>>      * Description: Oscillates between highest and lowest refresh each
>>> frame for
>>>>>>      *              manual flicker profiling
>>>>>> @@ -89,9 +94,10 @@ enum {
>>>>>>         TEST_FLIPLINE = 1 << 3,
>>>>>>         TEST_SEAMLESS_VRR = 1 << 4,
>>>>>>         TEST_SEAMLESS_DRRS = 1 << 5,
>>>>>> -     TEST_FASTSET = 1 << 6,
>>>>>> -     TEST_MAXMIN = 1 << 7,
>>>>>> -     TEST_NEGATIVE = 1 << 8,
>>>>>> +     TEST_SEAMLESS_VIRTUAL_RR = 1 << 6,
>>>>>> +     TEST_FASTSET = 1 << 7,
>>>>>> +     TEST_MAXMIN = 1 << 8,
>>>>>> +     TEST_NEGATIVE = 1 << 9,
>>>>>>     };
>>>>>>
>>>>>>     enum {
>>>>>> @@ -214,6 +220,18 @@ low_rr_mode_with_same_res(igt_output_t
>>> *output, unsigned int vrr_min)
>>>>>>         return mode;
>>>>>>     }
>>>>>>
>>>>>> +static drmModeModeInfo
>>>>>> +virtual_rr_vrr_range_mode(igt_output_t *output, unsigned int
>>>>>> +virtual_refresh_rate) {
>>>>>> +     drmModeModeInfo mode = *igt_output_get_mode(output);
>>>>>> +     uint64_t clock_hz = mode.clock * 1000;
>>>>>> +
>>>>>> +     mode.vtotal = clock_hz / (mode.htotal * virtual_refresh_rate);
>>>>>> +     mode.vrefresh = virtual_refresh_rate;
>>>>>> +
>>>>>> +     return mode;
>>>>>> +}
>>>>>> +
>>>>>>     /* Read min and max vrr range from the connector debugfs. */
>>>>>>     static range_t
>>>>>>     get_vrr_range(data_t *data, igt_output_t *output) @@ -641,6
>>>>>> +659,49 @@ test_seamless_rr_basic(data_t *data, enum pipe pipe,
>>> igt_output_t *output, uint3
>>>>>>                      vrr ? "on" : "off", vrr ? "not reached" : "exceeded", result);
>>>>>>     }
>>>>>>
>>>>>> +static void
>>>>>> +test_seamless_virtual_rr_basic(data_t *data, enum pipe pipe,
>>>>>> +igt_output_t *output, uint32_t flags) {
>>>>>> +     uint32_t result;
>>>>>> +     unsigned int vrefresh;
>>>>>> +     uint64_t rate[] = {0};
>>>>>> +
>>>>>> +     igt_info("Use HIGH_RR Mode as default\n");
>>>>>> +     kmstest_dump_mode(&data->switch_modes[HIGH_RR_MODE]);
>>>>>> +
>>>>>> +     prepare_test(data, output, pipe);
>>>>>> +     rate[0] =
>>>>>> + rate_from_refresh(data->switch_modes[HIGH_RR_MODE].vrefresh);
>>>>>> +
>>>>>> +     /*
>>>>>> +      * Sink with DRR and VRR can be in downclock mode so
>>>>>> +      * switch to highest refresh rate mode.
>>>>>> +      */
>>>>>> +     igt_output_override_mode(output, &data-
>>>> switch_modes[HIGH_RR_MODE]);
>>>>>> +     igt_assert(igt_display_try_commit_atomic(&data->display,
>>>>>> + DRM_MODE_PAGE_FLIP_EVENT, NULL) == 0);
>>>>>> +
>>>>>> +     result = flip_and_measure(data, output, pipe, rate, 1,
>>> TEST_DURATION_NS);
>>>>>> +     igt_assert_f(result > 75,
>>>>>> +                  "Refresh rate (%u Hz) %"PRIu64"ns: Target threshold not
>>> reached, result was %u%%\n",
>>>>>> +                  data->switch_modes[HIGH_RR_MODE].vrefresh,
>>>>>> + rate[0], result);
>>>>>> +
>>>>>> +     /* Switch to Virtual RR */
>>>>>> +     for (vrefresh = data->range.min + 10; vrefresh < data->range.max;
>>> vrefresh += 10) {
>>>>>> +             drmModeModeInfo virtual_mode =
>>>>>> + virtual_rr_vrr_range_mode(output, vrefresh);
>>>>>> +
>>>>>> +             igt_info("Requesting Virtual Mode with Refresh Rate (%u Hz): \n",
>>> vrefresh);
>>>>>> +             kmstest_dump_mode(&virtual_mode);
>>>>>> +
>>>>>> +             igt_output_override_mode(output, &virtual_mode);
>>>>>> +
>>>>>> + igt_assert(igt_display_try_commit_atomic(&data->display, 0, NULL)
>>>>>> + == 0);
>>>>>> +
>>>>>> +             rate[0] = rate_from_refresh(vrefresh);
>>>>>> +             result = flip_and_measure(data, output, pipe, rate, 1,
>>> TEST_DURATION_NS);
>>>>>> +             igt_assert_f(result > 75,
>>>>>> +                          "Refresh rate (%u Hz) %"PRIu64"ns: Target threshold not
>>> reached, result was %u%%\n",
>>>>>> +                          vrefresh, rate[0], result);
>>>>>> +     }
>>>>>> +}
>>>>>> +
>>>>>>     static void test_cleanup(data_t *data, enum pipe pipe, igt_output_t
>>> *output)
>>>>>>     {
>>>>>>         if (vrr_capable(output))
>>>>>> @@ -686,7 +747,7 @@ static bool output_constraint(data_t *data,
>>> igt_output_t *output, uint32_t flags
>>>>>>         igt_output_override_mode(output,
>>>>>> &data->switch_modes[HIGH_RR_MODE]);
>>>>>>
>>>>>>         /* Search for a low refresh rate mode. */
>>>>>> -     if (!(flags & (TEST_SEAMLESS_VRR | TEST_SEAMLESS_DRRS)))
>>>>>> +     if (!(flags & (TEST_SEAMLESS_VRR | TEST_SEAMLESS_DRRS |
>>>>>> + TEST_SEAMLESS_VIRTUAL_RR)))
>>>>>
>>>>> Does this virtual RR applicable to eDP only or supports other
>>>>> connector types too? If it is eDP specific [*], then we must add a check for
>>> that.
>>>>>
>>>>> [*]:
>>>>> https://cgit.freedesktop.org/drm/igt-gpu-tools/tree/tests/kms_vrr.c#n
>>>>> 660
>>>>>
>>>>> With above comment addressed, this patch is
>>>>> Reviewed-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
>>>>>
>>>>> - Bhanu
>>>>>
>>>>>>                 return true;
>>>>>>
>>>>>>         data->switch_modes[LOW_RR_MODE] =
>>>>>> low_rr_mode_with_same_res(output, data->range.min); @@ -841,6
>>> +902,10 @@ igt_main_args("drs:", long_opts, help_str, opt_handler, &data)
>>>>>>                              "between flip timestamps converges to the requested
>>> rate");
>>>>>>                 igt_subtest_with_dynamic("flip-basic-fastset")
>>>>>>                         run_vrr_test(&data, test_basic,
>>>>>> TEST_FASTSET);
>>>>>> +
>>>>>> +             igt_describe("Test to switch to any custom virtual mode in VRR
>>> range without modeset.");
>>>>>> +             igt_subtest_with_dynamic("seamless-rr-switch-virtual")
>>>>>> +                     run_vrr_test(&data,
>>>>>> + test_seamless_virtual_rr_basic, TEST_SEAMLESS_VIRTUAL_RR);
>>>>>>         }
>>>>>>
>>>>>>         igt_fixture {