[igt-dev] [PATCH] [i-g-t] tests/amdgpu/amd_psr: Fix eDP PSR1 test failure
Alex Hung
alex.hung at amd.com
Wed Jul 12 17:04:27 UTC 2023
Reviewed-by: Alex Hung <alex.hung at amd.com>
On 2023-07-12 07:30, Hersen Wu wrote:
> From: Hersen Wu <Hersenxs.Wu at amd.com>
>
> 1. Replace for_each_pipe_with_single_output with
> for_each_connected_output to avoid
> igt_display_commit_atomic return failure.
>
> Linux kernel AMD implementaion does not support dynamic
> allocatation for pipe connector combination. Kernel
> atomic check will return true only for one specific
> pipe connector combination, return false for other
> combinations.
>
> 2. Add eDP Rx PSR capability check psr_mode_supported.
>
> 3. Increase PSR_SETTLE_DELAY to 10 seconds.
> Measure Linux kernel AMD implementation, it will take around 8
> seconds for driver and firmmware to enter PSR1 state.
>
> Signed-off-by: Hersen Wu <Hersenxs.Wu at amd.com>
> ---
> lib/igt_amd.c | 4 +++-
> tests/amdgpu/amd_psr.c | 54 ++++++++++++++++++++++--------------------
> 2 files changed, 31 insertions(+), 27 deletions(-)
>
> diff --git a/lib/igt_amd.c b/lib/igt_amd.c
> index 8837714f7..8da405649 100644
> --- a/lib/igt_amd.c
> +++ b/lib/igt_amd.c
> @@ -1037,7 +1037,9 @@ bool igt_amd_psr_support_sink(int drm_fd, char *connector_name, enum psr_mode mo
> return false;
>
> if (mode == PSR_MODE_1)
> - return strstr(buf, "Sink support: yes [0x01]");
> + return strstr(buf, "Sink support: yes [0x01]") ||
> + strstr(buf, "Sink support: yes [0x03]") ||
> + strstr(buf, "Sink support: yes [0x04]");
> else
> return strstr(buf, "Sink support: yes [0x03]") ||
> strstr(buf, "Sink support: yes [0x04]");
> diff --git a/tests/amdgpu/amd_psr.c b/tests/amdgpu/amd_psr.c
> index 3932e143a..9da161a09 100644
> --- a/tests/amdgpu/amd_psr.c
> +++ b/tests/amdgpu/amd_psr.c
> @@ -38,8 +38,11 @@ IGT_TEST_DESCRIPTION("Basic test for enabling Panel Self Refresh for eDP display
>
> /* After a full update, a few fast updates are necessary for PSR to be enabled */
> #define N_FLIPS 6
> -/* DMCUB takes some time to actually enable PSR. Worst case delay is 4 seconds */
> -#define PSR_SETTLE_DELAY 4
> +/* Linux kernel AMD implementaion, After static image is submitted by kernel driver,
> + * kernel driver notify DMCUB FW ready to enter PSR1, DMUB DW takes around 8 seconds
> + * to actually enable PSR1 state.
> + */
> +#define PSR_SETTLE_DELAY 10
>
> typedef struct {
> int x, y;
> @@ -234,7 +237,7 @@ static int check_conn_type(data_t *data, uint32_t type) {
> return -1;
> }
>
> -static bool psr_su_supported(data_t *data)
> +static bool psr_mode_supported(data_t *data, enum psr_mode mode)
> {
> /* run PSR-SU test i.i.f. eDP panel and kernel driver both support PSR-SU */
> if (!igt_amd_output_has_psr_cap(data->fd, data->output->name)) {
> @@ -247,24 +250,24 @@ static bool psr_su_supported(data_t *data)
> return false;
> }
>
> - if (!igt_amd_psr_support_sink(data->fd, data->output->name, PSR_MODE_2)) {
> - igt_warn(" output %s not support PSR-SU\n", data->output->name);
> + if (!igt_amd_psr_support_sink(data->fd, data->output->name, mode)) {
> + igt_warn(" output %s not support PSR mode %d\n", data->output->name, mode);
> return false;
> }
>
> - if (!igt_amd_psr_support_drv(data->fd, data->output->name, PSR_MODE_2)) {
> - igt_warn(" kernel driver not support PSR-SU\n");
> + if (!igt_amd_psr_support_drv(data->fd, data->output->name, mode)) {
> + igt_warn(" kernel driver not support PSR mode %d\n", mode);
> return false;
> }
>
> return true;
> }
>
> +/* run_check_psr does not work with dual eDP */
> static void run_check_psr(data_t *data, bool test_null_crtc) {
> - int fd, edp_idx, dp_idx, ret, i, psr_state;
> + int edp_idx, dp_idx, ret, i, psr_state;
> igt_fb_t ref_fb, ref_fb2;
> igt_fb_t *flip_fb;
> - enum pipe pipe;
> igt_output_t *output;
>
> test_init(data);
> @@ -273,7 +276,12 @@ static void run_check_psr(data_t *data, bool test_null_crtc) {
> dp_idx = check_conn_type(data, DRM_MODE_CONNECTOR_DisplayPort);
> igt_skip_on_f(edp_idx == -1, "no eDP connector found\n");
>
> - for_each_pipe_with_single_output(&data->display, pipe, output) {
> + /* check if eDP support PSR1, PSR-SU.
> + * test_null_crtc test is valid only with PSR eDP connected
> + */
> + 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;
>
> @@ -285,7 +293,7 @@ static void run_check_psr(data_t *data, bool test_null_crtc) {
> 1.0, 0.0, &ref_fb2);
>
> igt_plane_set_fb(data->primary, &ref_fb);
> - igt_output_set_pipe(output, pipe);
> +
> igt_display_commit_atomic(&data->display, DRM_MODE_ATOMIC_ALLOW_MODESET, 0);
>
> for (i = 0; i < N_FLIPS; i++) {
> @@ -299,29 +307,26 @@ static void run_check_psr(data_t *data, bool test_null_crtc) {
> 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);
> -
> - for_each_pipe_with_single_output(&data->display, pipe, output) {
> - if (output->config.connector->connector_type != DRM_MODE_CONNECTOR_eDP)
> - continue;
> + /* 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_fail_on_f(psr_state < PSR_STATE0, "Open PSR state debugfs failed\n");
> igt_fail_on_f(psr_state < PSR_STATE1, "PSR was not enabled for connector %s\n", output->name);
> igt_fail_on_f(psr_state == PSR_STATE_INVALID, "PSR is invalid for connector %s\n", output->name);
> igt_fail_on_f(psr_state != PSR_STATE3, "PSR state is expected to be at PSR_STATE3 (Active) on a "
> -
> "static screen for connector %s\n", output->name);
> +
> + igt_remove_fb(data->fd, &ref_fb);
> + igt_remove_fb(data->fd, &ref_fb2);
> }
>
> if (test_null_crtc) {
> /* check whether settings crtc to null generated any warning (eDP+DP) */
> igt_skip_on_f(dp_idx == -1, "no DP connector found\n");
>
> - for_each_pipe_with_single_output(&data->display, pipe, output) {
> + for_each_connected_output(&data->display, output) {
> if (output->config.connector->connector_type != DRM_MODE_CONNECTOR_DisplayPort)
> continue;
>
> @@ -330,9 +335,6 @@ static void run_check_psr(data_t *data, bool test_null_crtc) {
> }
> }
>
> - igt_remove_fb(data->fd, &ref_fb);
> - igt_remove_fb(data->fd, &ref_fb2);
> - close(fd);
> test_fini(data);
> }
>
> @@ -363,7 +365,7 @@ static void run_check_psr_su_mpo(data_t *data, bool scaling, float scaling_ratio
> }
>
> /* run the test i.i.f. eDP panel supports and kernel driver both support PSR-SU */
> - igt_skip_on(!psr_su_supported(data));
> + igt_skip_on(!psr_mode_supported(data, PSR_MODE_2));
>
> /* reference background pattern in grey */
> igt_create_color_fb(data->fd, data->w, data->h, DRM_FORMAT_XRGB8888, DRM_FORMAT_MOD_LINEAR,
> @@ -504,7 +506,7 @@ static void run_check_psr_su_ffu(data_t *data)
> test_init(data);
>
> /* run the test i.i.f. eDP panel supports and kernel driver both support PSR-SU */
> - igt_skip_on(!psr_su_supported(data));
> + igt_skip_on(!psr_mode_supported(data, PSR_MODE_2));
>
> /* reference background pattern in grey */
> igt_create_color_fb(data->fd, data->w, data->h, DRM_FORMAT_XRGB8888, DRM_FORMAT_MOD_LINEAR,
> @@ -616,7 +618,7 @@ static void run_check_psr_su_cursor(data_t *data, bool test_mpo)
> igt_skip_on_f(edp_idx == -1, "no eDP connector found\n");
>
> test_init(data);
> - igt_skip_on(!psr_su_supported(data));
> + igt_skip_on(!psr_mode_supported(data, PSR_MODE_2));
>
> frame_rate = data->mode->vrefresh;
>
More information about the igt-dev
mailing list