Re: [PATCH i-g-t 1/2] tests/amdgpu/amd_replay: fix operator precedence and typos

[Alex Hung <alex.hung@amd.com>]

Reviewed-by: Alex Hung <alex.hung@amd.com>


On 5/12/26 01:59, Ray Wu wrote:
> [Why]
> Several existing replay subtests had logic bugs that silently disabled
> their state checks:
> 
>    - `test_mode == (A || B)` and `test_mode == A || B` do not check
>      whether test_mode matches A or B; both evaluate to a constant
>      truthy expression, so the guarded code always ran.
>    - Three igt_fail_on_f() range checks were written as
>      `state < N && state >= N+1`, which can never be true; the
>      assertions never fired regardless of replay_state.
> 
> Two "mdoe" typos in igt_describe() strings were also present.
> 
> [How]
>    - Rewrite the conditionals as `(test_mode == A) || (test_mode == B)`.
>    - Replace `&&` with `||` in the three range assertions so they fail
>      when state is outside the expected range.
>    - Fix the two typos.
> 
> Signed-off-by: Ray Wu <ray.wu@amd.com>
> ---
>   tests/amdgpu/amd_replay.c | 28 +++++++++++++++++-----------
>   1 file changed, 17 insertions(+), 11 deletions(-)
> 
> diff --git a/tests/amdgpu/amd_replay.c b/tests/amdgpu/amd_replay.c
> index 775bed190..4b795f5ba 100644
> --- a/tests/amdgpu/amd_replay.c
> +++ b/tests/amdgpu/amd_replay.c
> @@ -236,14 +236,17 @@ static void page_flip_test(struct test_data *data, igt_output_t *output,
>   		igt_require(ret == 0);
>   		kmstest_wait_for_pageflip(data->fd);
>   
> -		if (test_mode == (TEST_MODE_CONSTANT_LIVE || TEST_MODE_INTERMITTENT_LIVE)
> -				&& frame_count > 5) {
> +		if ((test_mode == TEST_MODE_CONSTANT_LIVE ||
> +		     test_mode == TEST_MODE_INTERMITTENT_LIVE) &&
> +		    frame_count > 5) {
>   			/* Panel Replay state needs few frame to enter the live mode */
>   			replay_state = igt_amd_read_replay_state(data->fd, output->name);
>   			dpcd_read_byte(data->fd, output->config.connector, 0x378, &panel_dpcd);
>   			igt_debug("replay_state live mode = 0x%X\n", replay_state);
> -			igt_fail_on_f(replay_state < REPLAY_STATE_4 && replay_state >= REPLAY_STATE_5,
> -					"State should be REPLAY_STATE_4 (Active with single frame update)\n");
> +			igt_fail_on_f(replay_state < REPLAY_STATE_4 ||
> +				      replay_state >= REPLAY_STATE_5,
> +				      "State should be REPLAY_STATE_4 "
> +				      "(Active with single frame update)\n");
>   			igt_fail_on_f(panel_dpcd == 0, "Panel is not in replay mode\n");
>   		}
>   
> @@ -302,15 +305,17 @@ static void run_check_replay(struct test_data *data, enum test_mode test_mode)
>   		page_flip_test(data, output, test_mode, 20);
>   
>   		/* Check Panel Replay state in static screen */
> -		if (test_mode == TEST_MODE_STATIC_SCREEN || TEST_MODE_INTERMITTENT_LIVE) {
> +		if (test_mode == TEST_MODE_STATIC_SCREEN ||
> +		    test_mode == TEST_MODE_INTERMITTENT_LIVE) {
>   			/* Panel Replay state takes some time to settle its value on static screen */
>   			sleep(1);
>   
>   			replay_state = igt_amd_read_replay_state(data->fd, output->name);
>   			dpcd_read_byte(data->fd, output->config.connector, 0x378, &panel_dpcd);
>   			igt_debug("replay_state static mode = 0x%X\n", replay_state);
> -			igt_fail_on_f(replay_state < REPLAY_STATE_3 && replay_state >= REPLAY_STATE_4,
> -				 "State should be REPLAY_STATE_3 (Active)\n");
> +			igt_fail_on_f(replay_state < REPLAY_STATE_3 ||
> +				      replay_state >= REPLAY_STATE_4,
> +				      "State should be REPLAY_STATE_3 (Active)\n");
>   			igt_fail_on_f(panel_dpcd == 0, "Panel is not in replay mode\n");
>   		}
>   
> @@ -325,8 +330,9 @@ static void run_check_replay(struct test_data *data, enum test_mode test_mode)
>   			dpcd_read_byte(data->fd, output->config.connector, 0x378, &panel_dpcd);
>   			igt_debug("replay_state TEST_MODE_INTERMITTENT_LIVE after flip = 0x%X\n",
>   				 replay_state);
> -			igt_fail_on_f(replay_state < REPLAY_STATE_3 && replay_state >= REPLAY_STATE_4,
> -				 "State should be REPLAY_STATE_3 (Active)\n");
> +			igt_fail_on_f(replay_state < REPLAY_STATE_3 ||
> +				      replay_state >= REPLAY_STATE_4,
> +				      "State should be REPLAY_STATE_3 (Active)\n");
>   			igt_fail_on_f(panel_dpcd == 0, "Panel is not in replay mode\n");
>   		}
>   
> @@ -439,10 +445,10 @@ int igt_main_args("", long_options, help_str, opt_handler, NULL)
>   	igt_describe("Test whether Panel Replay can be enabled with static screen");
>   	igt_subtest("replay_static_screen") run_check_replay(&data, TEST_MODE_STATIC_SCREEN);
>   
> -	igt_describe("Test whether Panel Replay can be enabled with intermittent live mdoe");
> +	igt_describe("Test whether Panel Replay can be enabled with intermittent live mode");
>   	igt_subtest("replay_intermittent_live") run_check_replay(&data, TEST_MODE_INTERMITTENT_LIVE);
>   
> -	igt_describe("Test whether Panel Replay can be enabled with constant live mdoe");
> +	igt_describe("Test whether Panel Replay can be enabled with constant live mode");
>   	igt_subtest("replay_constant_live") run_check_replay(&data, TEST_MODE_CONSTANT_LIVE);
>   
>   	igt_describe("Test whether Panel Replay can be enabled after resume from suspend");