[igt-dev] [PATCH] [i-g-t] tests/amdgpu/amd_vrr_range: fix test crash when n_pipes > n_outputs

[Wu, Hersen]

[AMD Official Use Only - General]

Hi Alex,

Yes. Test crashes if there are more pipes than outputs.

n_pipes, n_outputs are defined within igt_kms.h

struct igt_display {
        int drm_fd;
        int log_shift;
        int n_pipes;                        ---->   display->n_pipes =  resources->count_crtcs   IGT_MAX_PIPES;
        int n_planes;                      ---->   display->n_planes = plane_resources->count_planes;
        int n_outputs;                    ---->   display->n_outputs = resources->count_connectors;
        igt_output_t *outputs;
        igt_plane_t *planes;
        igt_pipe_t *pipes;
        bool has_cursor_plane;
        bool is_atomic;
        bool first_commit;

        uint64_t *modifiers;
        uint32_t *formats;
        int format_mod_count;
};

Thanks!
Hersen



-----Original Message-----
From: Hung, Alex <Alex.Hung at amd.com>
Sent: Tuesday, June 13, 2023 1:23 PM
To: Wu, Hersen <hersenxs.wu at amd.com>; igt-dev at lists.freedesktop.org; Siqueira, Rodrigo <Rodrigo.Siqueira at amd.com>; Pillai, Aurabindo <Aurabindo.Pillai at amd.com>; Wang, Chao-kai (Stylon) <Stylon.Wang at amd.com>; Mahfooz, Hamza <Hamza.Mahfooz at amd.com>
Subject: Re: [PATCH] [i-g-t] tests/amdgpu/amd_vrr_range: fix test crash when n_pipes > n_outputs

The subject and commit messages imply there are variables "n_pipes" and "n_outputs", but there aren't such variables.

Do you mean the test crashes if there are more pipes than outputs?

On 2023-06-13 06:46, Hersen Wu wrote:
> Fix test crash by looping outputs using
> for_each_connected_output(display, output).
>
> When n_pipes > n_outputs, i >= n_outputs,
> display->outputs[i]->config.connector within
> for_each_pipe(display, i) are invalid.
> Invalid connectors cause test crash.
>
> Signed-off-by: Hersen Wu <hersenxs.wu at amd.com>
> ---
>   tests/amdgpu/amd_vrr_range.c | 20 ++++++--------------
>   1 file changed, 6 insertions(+), 14 deletions(-)
>
> diff --git a/tests/amdgpu/amd_vrr_range.c
> b/tests/amdgpu/amd_vrr_range.c index 3f8f23e9a..611f41f95 100644
> --- a/tests/amdgpu/amd_vrr_range.c
> +++ b/tests/amdgpu/amd_vrr_range.c
> @@ -258,18 +258,14 @@ static void test_freesync_parsing_base(data_t *data, uint32_t test_flags)
>       const struct edid *edid;
>       range_t range, expected_range;
>       igt_output_t *output;
> -     int i, j, test_conn_cnt = 0;
> +     int j, test_conn_cnt = 0;
> +     igt_display_t *display = &data->display;
>
>       test_init(data);
>
>       igt_amd_require_hpd(&data->display, data->fd);
>
> -     for_each_pipe(&data->display, i) {
> -             /* setup the output */
> -             output = data->output[i];
> -             if (!output || !igt_output_is_connected(output))
> -                     continue;
> -
> +     for_each_connected_output(display, output) {
>               /* find a test EDID */
>               j =
> find_test_edid_index(output->config.connector->connector_type);
>
> @@ -335,18 +331,14 @@ static void test_freesync_range_base(data_t *data, uint32_t test_flags)
>   {
>       range_t range;
>       igt_output_t *output;
> -     int i, test_conn_cnt = 0;
> +     int test_conn_cnt = 0;
> +     igt_display_t *display = &data->display;
>
>       test_init(data);
>
>       igt_amd_require_hpd(&data->display, data->fd);
>
> -     for_each_pipe(&data->display, i) {
> -             /* setup the output */
> -             output = data->output[i];
> -             if (!output || !igt_output_is_connected(output))
> -                     continue;
> -
> +     for_each_connected_output(display, output) {
>               igt_debug_on_f(!has_vrr(output), "Requires output supports
> VRR\n");
>
>               if (!has_vrr(output)) {