Re: [PATCH i-g-t v1] tests/kms_async_flips: Add psr async flip subtest

["Reddy Guddati, Santhosh" <santhosh.reddy.guddati@intel.com>]

Hi Karthik,

On 28-03-2025 09:28, Karthik B S wrote:
> Hi Santhosh,
> 
> On 3/24/2025 9:43 PM, Santhosh Reddy Guddati wrote:
>> Add a new subtest to verify async flips does not cause PSR exit.
>> Enable PSR and execute async flips to verify system remains in
>> PSR mode after async flips.
>>
>> Signed-off-by: Santhosh Reddy Guddati <santhosh.reddy.guddati@intel.com>
>> ---
>>   tests/kms_async_flips.c | 38 ++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 38 insertions(+)
>>
>> diff --git a/tests/kms_async_flips.c b/tests/kms_async_flips.c
>> index da426f753..cf6b63b3e 100644
>> --- a/tests/kms_async_flips.c
>> +++ b/tests/kms_async_flips.c
>> @@ -85,6 +85,9 @@
>>    *
>>    * SUBTEST: async-flip-suspend-resume
>>    * Description: Verify the async flip functionality with suspend and 
>> resume cycle
>> + *
>> + * SUBTEST: psr-async-flip
>> + * Description: Verify that async flips do not cause PSR exit
>>    */
>>   #define CURSOR_POS 128
>> @@ -102,6 +105,7 @@ IGT_TEST_DESCRIPTION("Test asynchronous page 
>> flips.");
>>   typedef struct {
>>       int drm_fd;
>> +    int debugfs_fd;
>>       uint32_t crtc_id;
>>       uint32_t refresh_rate;
>>       struct igt_fb bufs[NUM_FBS];
>> @@ -740,6 +744,33 @@ static void run_test_with_modifiers(data_t *data, 
>> void (*test)(data_t *))
>>       }
>>   }
>> +static bool psr_wait_entry_if_enabled(data_t *data)
>> +{
>> +    igt_skip_on_f(!is_psr_enable_possible(data->drm_fd, PSR_MODE_1),
>> +              "enable_psr modparam doesn't allow PSR mode 1\n");
>> +
>> +    return psr_wait_entry(data->debugfs_fd, PSR_MODE_1, data->output);
>> +}
>> +
>> +static void test_psr_async_flip(data_t *data)
>> +{
>> +    if (!psr_sink_support(data->drm_fd, data->debugfs_fd, PSR_MODE_1, 
>> data->output))
>> +        igt_skip("PSR mode 1 is not supported on this output\n");
>> +
>> +    psr_enable(data->drm_fd, data->debugfs_fd, PSR_MODE_1, NULL);
>> +    igt_display_commit(&data->display);
>> +
>> +    /* Confirm PSR entry before starting async flips */
>> +    igt_assert_f(psr_wait_entry_if_enabled(data),
>> +             "PSR is not enabled before async flip test\n");
>> +
>> +    test_async_flip(data);
>> +
>> +    /* Confirm PSR is still active after async flips */
>> +    igt_assert_f(psr_wait_entry_if_enabled(data),
>> +             "PSR is not enabled after async flip test\n");
> 
> We need to be using the 'psr_active_check' here directly instead of 
> waiting for PSR entry as this contradicts the comment above the check.
> 
> This test will mostly fail once we have that check async flips should 
> ideally trigger PSR exit?
> 

I have used psr_active_check and verified that async flips are 
triggering PSR exit, causing the test to fail.

Given this information, I agree that the test will not work as the async 
flips cause PSR to exit. so, I am rejecting this patch.

> Thanks,
> Karthik.B.S
>> +}
>> +
>>   static data_t data;
>>   igt_main
>> @@ -748,6 +779,7 @@ igt_main
>>       igt_fixture {
>>           data.drm_fd = drm_open_driver_master(DRIVER_ANY);
>> +        data.debugfs_fd = igt_debugfs_dir(data.drm_fd);
>>           kmstest_set_vt_graphics_mode();
>>           igt_display_require(&data.display, data.drm_fd);
>>           igt_display_require_output(&data.display);
>> @@ -896,6 +928,12 @@ igt_main
>>           run_test(&data, test_async_flip);
>>       }
>> +    igt_describe("Verify that async flips do not cause PSR exit");
>> +    igt_subtest_with_dynamic("psr-async-flip") {
>> +        data.atomic_path = false;
>> +        run_test(&data, test_psr_async_flip);
>> +    }
>> +
>>       igt_fixture {
>>           for (i = 0; i < NUM_FBS; i++)
>>               igt_remove_fb(data.drm_fd, &data.bufs[i]);