diff mbox series

[v2,4/4] tests/amdgpu/amd_psr: Add support for `power saving policy` property

Message ID 20240522220849.33343-5-mario.limonciello@amd.com (mailing list archive)
State New, archived
Headers show
Series Add support for testing power saving policy DRM property | expand

Commit Message

Mario Limonciello May 22, 2024, 10:08 p.m. UTC 
Verify that the property has disabled PSR
---
 tests/amdgpu/amd_psr.c | 74 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 74 insertions(+)

Comments

Leo Li June 18, 2024, 8:20 p.m. UTC | #1 
On 2024-05-22 18:08, Mario Limonciello wrote:
> Verify that the property has disabled PSR
> ---
>   tests/amdgpu/amd_psr.c | 74 ++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 74 insertions(+)
> 
> diff --git a/tests/amdgpu/amd_psr.c b/tests/amdgpu/amd_psr.c
> index 9da161a09..a9f4a6aa5 100644
> --- a/tests/amdgpu/amd_psr.c
> +++ b/tests/amdgpu/amd_psr.c
> @@ -338,6 +338,78 @@ static void run_check_psr(data_t *data, bool test_null_crtc) {
>   	test_fini(data);
>   }
>   
> +static void psr_forbidden(data_t *data)
> +{
> +	int edp_idx, ret, i, psr_state;
> +	igt_fb_t ref_fb, ref_fb2;
> +	igt_fb_t *flip_fb;
> +	igt_output_t *output;
> +
> +	test_init(data);
> +
> +	edp_idx = check_conn_type(data, DRM_MODE_CONNECTOR_eDP);
> +	igt_skip_on_f(edp_idx == -1, "no eDP connector found\n");
> +
> +	/* check if eDP support PSR1, PSR-SU.
> +	 */
> +	igt_skip_on(!psr_mode_supported(data, PSR_MODE_1) && !psr_mode_supported(data, PSR_MODE_2));
> +	for_each_connected_output(&data->display, output) {
> +
> +		if (output->config.connector->connector_type != DRM_MODE_CONNECTOR_eDP)
> +			continue;
> +		ret = clear_power_saving_policy(data->fd, output);
> +		if (ret == -ENODEV) {
> +			igt_skip("No power saving policy prop\n");
> +			return;
> +		}
> +		igt_require(ret == 0);
> +
> +		/* try to engage PSR */
> +		ret = set_panel_power_saving_policy(data->fd, output, DRM_MODE_REQUIRE_LOW_LATENCY);
> +		igt_assert_eq(ret, 0);
> +
> +		igt_create_color_fb(data->fd, data->mode->hdisplay,
> +				    data->mode->vdisplay, DRM_FORMAT_XRGB8888, 0, 1.0,
> +				    0.0, 0.0, &ref_fb);
> +		igt_create_color_fb(data->fd, data->mode->hdisplay,
> +				    data->mode->vdisplay, DRM_FORMAT_XRGB8888, 0, 0.0,
> +				    1.0, 0.0, &ref_fb2);
> +
> +		igt_plane_set_fb(data->primary, &ref_fb);
> +
> +		igt_display_commit_atomic(&data->display, DRM_MODE_ATOMIC_ALLOW_MODESET, 0);
> +
> +		for (i = 0; i < N_FLIPS; i++) {
> +			if (i % 2 == 0)
> +				flip_fb = &ref_fb2;
> +			else
> +				flip_fb = &ref_fb;
> +
> +			ret = drmModePageFlip(data->fd, output->config.crtc->crtc_id,
> +					      flip_fb->fb_id, DRM_MODE_PAGE_FLIP_EVENT, NULL);
> +			igt_require(ret == 0);
> +			kmstest_wait_for_pageflip(data->fd);
> +		}
> +
> +		/* PSR state takes some time to settle its value on static screen */
> +		sleep(PSR_SETTLE_DELAY);
> +
> +		psr_state =  igt_amd_read_psr_state(data->fd, output->name);
> +		igt_require(psr_state == PSR_STATE3);

igt_fail_on* or igt_assert* should be used here, igt_require simply 'skips' the
test if the condition evaluates to false.

Should we be instead asserting psr_state == PSR_STATE0 here for disabled, since
we've set REQUIRE_LOW_LATENCY?

I think as part of this test, we can also check that PSR re-enables after
clearing the power saving policy. Something like

ret = clear_power_saving_policy(data->fd, output);
... do some flipping ...
sleep(PSR_SETTLE_DELAY);
psr_state = igt_amd_read_psr_state(data->fd, output->name);
igt_assert_f(psr_state == PSR_STATE3,
              "Panel not in PSR after clearing power saving policy\n");

Thanks,
Leo

> +
> +		igt_remove_fb(data->fd, &ref_fb);
> +		igt_remove_fb(data->fd, &ref_fb2);
> +
> +		ret = clear_power_saving_policy(data->fd, output);
> +		if (ret == -ENODEV) {
> +			igt_skip("No power saving policy prop\n");
> +			return;
> +		}
> +		igt_require(ret == 0);
> +
> +	}
> +}
> +
>   static void run_check_psr_su_mpo(data_t *data, bool scaling, float scaling_ratio)
>   {
>   	int edp_idx = check_conn_type(data, DRM_MODE_CONNECTOR_eDP);
> @@ -756,6 +828,8 @@ igt_main_args("", long_options, help_str, opt_handler, NULL)
>   	igt_describe("Test to validate PSR SU enablement with Visual Confirm "
>   		     "and to validate PSR SU disable/re-enable w/ primary scaling ratio 0.75");
>   	igt_subtest("psr_su_mpo_scaling_0_75") run_check_psr_su_mpo(&data, true, .75);
> +	igt_describe("Test whether PSR can be forbidden");
> +	igt_subtest("psr_forbidden") psr_forbidden(&data);
>   
>   	igt_fixture
>   	{
Mario Limonciello June 18, 2024, 9:07 p.m. UTC | #2 
On 6/18/2024 15:20, Leo Li wrote:
> 
> 
> On 2024-05-22 18:08, Mario Limonciello wrote:
>> Verify that the property has disabled PSR
>> ---
>>   tests/amdgpu/amd_psr.c | 74 ++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 74 insertions(+)
>>
>> diff --git a/tests/amdgpu/amd_psr.c b/tests/amdgpu/amd_psr.c
>> index 9da161a09..a9f4a6aa5 100644
>> --- a/tests/amdgpu/amd_psr.c
>> +++ b/tests/amdgpu/amd_psr.c
>> @@ -338,6 +338,78 @@ static void run_check_psr(data_t *data, bool 
>> test_null_crtc) {
>>       test_fini(data);
>>   }
>> +static void psr_forbidden(data_t *data)
>> +{
>> +    int edp_idx, ret, i, psr_state;
>> +    igt_fb_t ref_fb, ref_fb2;
>> +    igt_fb_t *flip_fb;
>> +    igt_output_t *output;
>> +
>> +    test_init(data);
>> +
>> +    edp_idx = check_conn_type(data, DRM_MODE_CONNECTOR_eDP);
>> +    igt_skip_on_f(edp_idx == -1, "no eDP connector found\n");
>> +
>> +    /* check if eDP support PSR1, PSR-SU.
>> +     */
>> +    igt_skip_on(!psr_mode_supported(data, PSR_MODE_1) && 
>> !psr_mode_supported(data, PSR_MODE_2));
>> +    for_each_connected_output(&data->display, output) {
>> +
>> +        if (output->config.connector->connector_type != 
>> DRM_MODE_CONNECTOR_eDP)
>> +            continue;
>> +        ret = clear_power_saving_policy(data->fd, output);
>> +        if (ret == -ENODEV) {
>> +            igt_skip("No power saving policy prop\n");
>> +            return;
>> +        }
>> +        igt_require(ret == 0);
>> +
>> +        /* try to engage PSR */
>> +        ret = set_panel_power_saving_policy(data->fd, output, 
>> DRM_MODE_REQUIRE_LOW_LATENCY);
>> +        igt_assert_eq(ret, 0);
>> +
>> +        igt_create_color_fb(data->fd, data->mode->hdisplay,
>> +                    data->mode->vdisplay, DRM_FORMAT_XRGB8888, 0, 1.0,
>> +                    0.0, 0.0, &ref_fb);
>> +        igt_create_color_fb(data->fd, data->mode->hdisplay,
>> +                    data->mode->vdisplay, DRM_FORMAT_XRGB8888, 0, 0.0,
>> +                    1.0, 0.0, &ref_fb2);
>> +
>> +        igt_plane_set_fb(data->primary, &ref_fb);
>> +
>> +        igt_display_commit_atomic(&data->display, 
>> DRM_MODE_ATOMIC_ALLOW_MODESET, 0);
>> +
>> +        for (i = 0; i < N_FLIPS; i++) {
>> +            if (i % 2 == 0)
>> +                flip_fb = &ref_fb2;
>> +            else
>> +                flip_fb = &ref_fb;
>> +
>> +            ret = drmModePageFlip(data->fd, 
>> output->config.crtc->crtc_id,
>> +                          flip_fb->fb_id, DRM_MODE_PAGE_FLIP_EVENT, 
>> NULL);
>> +            igt_require(ret == 0);
>> +            kmstest_wait_for_pageflip(data->fd);
>> +        }
>> +
>> +        /* PSR state takes some time to settle its value on static 
>> screen */
>> +        sleep(PSR_SETTLE_DELAY);
>> +
>> +        psr_state =  igt_amd_read_psr_state(data->fd, output->name);
>> +        igt_require(psr_state == PSR_STATE3);
> 
> igt_fail_on* or igt_assert* should be used here, igt_require simply 
> 'skips' the
> test if the condition evaluates to false.
> 

Got it; thanks.

> Should we be instead asserting psr_state == PSR_STATE0 here for 
> disabled, since
> we've set REQUIRE_LOW_LATENCY?

Yeah I think you're right.

> 
> I think as part of this test, we can also check that PSR re-enables after
> clearing the power saving policy. Something like
> 
> ret = clear_power_saving_policy(data->fd, output);
> ... do some flipping ...
> sleep(PSR_SETTLE_DELAY);
> psr_state = igt_amd_read_psr_state(data->fd, output->name);
> igt_assert_f(psr_state == PSR_STATE3,
>               "Panel not in PSR after clearing power saving policy\n");
> 

Agree, thanks.

> Thanks,
> Leo
> 
>> +
>> +        igt_remove_fb(data->fd, &ref_fb);
>> +        igt_remove_fb(data->fd, &ref_fb2);
>> +
>> +        ret = clear_power_saving_policy(data->fd, output);
>> +        if (ret == -ENODEV) {
>> +            igt_skip("No power saving policy prop\n");
>> +            return;
>> +        }
>> +        igt_require(ret == 0);
>> +
>> +    }
>> +}
>> +
>>   static void run_check_psr_su_mpo(data_t *data, bool scaling, float 
>> scaling_ratio)
>>   {
>>       int edp_idx = check_conn_type(data, DRM_MODE_CONNECTOR_eDP);
>> @@ -756,6 +828,8 @@ igt_main_args("", long_options, help_str, 
>> opt_handler, NULL)
>>       igt_describe("Test to validate PSR SU enablement with Visual 
>> Confirm "
>>                "and to validate PSR SU disable/re-enable w/ primary 
>> scaling ratio 0.75");
>>       igt_subtest("psr_su_mpo_scaling_0_75") 
>> run_check_psr_su_mpo(&data, true, .75);
>> +    igt_describe("Test whether PSR can be forbidden");
>> +    igt_subtest("psr_forbidden") psr_forbidden(&data);
>>       igt_fixture
>>       {
diff mbox series

Patch

diff --git a/tests/amdgpu/amd_psr.c b/tests/amdgpu/amd_psr.c
index 9da161a09..a9f4a6aa5 100644
--- a/tests/amdgpu/amd_psr.c
+++ b/tests/amdgpu/amd_psr.c
@@ -338,6 +338,78 @@  static void run_check_psr(data_t *data, bool test_null_crtc) {
 	test_fini(data);
 }
 
+static void psr_forbidden(data_t *data)
+{
+	int edp_idx, ret, i, psr_state;
+	igt_fb_t ref_fb, ref_fb2;
+	igt_fb_t *flip_fb;
+	igt_output_t *output;
+
+	test_init(data);
+
+	edp_idx = check_conn_type(data, DRM_MODE_CONNECTOR_eDP);
+	igt_skip_on_f(edp_idx == -1, "no eDP connector found\n");
+
+	/* check if eDP support PSR1, PSR-SU.
+	 */
+	igt_skip_on(!psr_mode_supported(data, PSR_MODE_1) && !psr_mode_supported(data, PSR_MODE_2));
+	for_each_connected_output(&data->display, output) {
+
+		if (output->config.connector->connector_type != DRM_MODE_CONNECTOR_eDP)
+			continue;
+		ret = clear_power_saving_policy(data->fd, output);
+		if (ret == -ENODEV) {
+			igt_skip("No power saving policy prop\n");
+			return;
+		}
+		igt_require(ret == 0);
+
+		/* try to engage PSR */
+		ret = set_panel_power_saving_policy(data->fd, output, DRM_MODE_REQUIRE_LOW_LATENCY);
+		igt_assert_eq(ret, 0);
+
+		igt_create_color_fb(data->fd, data->mode->hdisplay,
+				    data->mode->vdisplay, DRM_FORMAT_XRGB8888, 0, 1.0,
+				    0.0, 0.0, &ref_fb);
+		igt_create_color_fb(data->fd, data->mode->hdisplay,
+				    data->mode->vdisplay, DRM_FORMAT_XRGB8888, 0, 0.0,
+				    1.0, 0.0, &ref_fb2);
+
+		igt_plane_set_fb(data->primary, &ref_fb);
+
+		igt_display_commit_atomic(&data->display, DRM_MODE_ATOMIC_ALLOW_MODESET, 0);
+
+		for (i = 0; i < N_FLIPS; i++) {
+			if (i % 2 == 0)
+				flip_fb = &ref_fb2;
+			else
+				flip_fb = &ref_fb;
+
+			ret = drmModePageFlip(data->fd, output->config.crtc->crtc_id,
+					      flip_fb->fb_id, DRM_MODE_PAGE_FLIP_EVENT, NULL);
+			igt_require(ret == 0);
+			kmstest_wait_for_pageflip(data->fd);
+		}
+
+		/* PSR state takes some time to settle its value on static screen */
+		sleep(PSR_SETTLE_DELAY);
+
+		psr_state =  igt_amd_read_psr_state(data->fd, output->name);
+		igt_require(psr_state == PSR_STATE3);
+
+		igt_remove_fb(data->fd, &ref_fb);
+		igt_remove_fb(data->fd, &ref_fb2);
+
+		ret = clear_power_saving_policy(data->fd, output);
+		if (ret == -ENODEV) {
+			igt_skip("No power saving policy prop\n");
+			return;
+		}
+		igt_require(ret == 0);
+
+	}
+}
+
 static void run_check_psr_su_mpo(data_t *data, bool scaling, float scaling_ratio)
 {
 	int edp_idx = check_conn_type(data, DRM_MODE_CONNECTOR_eDP);
@@ -756,6 +828,8 @@  igt_main_args("", long_options, help_str, opt_handler, NULL)
 	igt_describe("Test to validate PSR SU enablement with Visual Confirm "
 		     "and to validate PSR SU disable/re-enable w/ primary scaling ratio 0.75");
 	igt_subtest("psr_su_mpo_scaling_0_75") run_check_psr_su_mpo(&data, true, .75);
+	igt_describe("Test whether PSR can be forbidden");
+	igt_subtest("psr_forbidden") psr_forbidden(&data);
 
 	igt_fixture
 	{