From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from NAM12-MW2-obe.outbound.protection.outlook.com (mail-mw2nam12on2085.outbound.protection.outlook.com [40.107.244.85]) by gabe.freedesktop.org (Postfix) with ESMTPS id 2130910E412 for ; Wed, 14 Dec 2022 15:07:43 +0000 (UTC) From: "Hung, Alex" To: Petri Latvala Date: Wed, 14 Dec 2022 15:07:41 +0000 Message-ID: References: <20221213234332.364224-1-alex.hung@amd.com> <20221213234332.364224-2-alex.hung@amd.com> In-Reply-To: Content-Language: en-GB Content-Type: multipart/alternative; boundary="_000_BYAPR12MB3048BC33EEB684DC1A14DFD0F7E09BYAPR12MB3048namp_" MIME-Version: 1.0 Subject: Re: [igt-dev] [PATCH 2/2] tests/kms_bw: Run tests on available pipes only List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: "igt-dev@lists.freedesktop.org" Errors-To: igt-dev-bounces@lists.freedesktop.org Sender: "igt-dev" List-ID: --_000_BYAPR12MB3048BC33EEB684DC1A14DFD0F7E09BYAPR12MB3048namp_ Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable [AMD Official Use Only - General] Thanks for the feedback. I will drop patch 2 ________________________________ From: Petri Latvala Sent: 14 December 2022 01:30 To: Hung, Alex Cc: igt-dev@lists.freedesktop.org Subject: Re: [igt-dev] [PATCH 2/2] tests/kms_bw: Run tests on available pip= es only On Tue, Dec 13, 2022 at 04:43:32PM -0700, Alex Hung wrote: > This cleans up unnecessary skips during executions. > > Signed-off-by: Alex Hung > --- > tests/kms_bw.c | 16 ++++------------ > 1 file changed, 4 insertions(+), 12 deletions(-) > > diff --git a/tests/kms_bw.c b/tests/kms_bw.c > index 27100587..ac801746 100644 > --- a/tests/kms_bw.c > +++ b/tests/kms_bw.c > @@ -145,17 +145,9 @@ static void run_test_linear_tiling(data_t *data, int= pipe, const drmModeModeInfo > igt_output_t *output; > struct igt_fb buffer[IGT_MAX_PIPES]; > igt_crc_t zero, captured[IGT_MAX_PIPES]; > - int i =3D 0, num_pipes =3D 0; > - enum pipe p; > + int i =3D 0; > int ret; > > - /* Cannot use igt_display_get_n_pipes() due to fused pipes on i915 = where they do > - * not give the numver of valid crtcs and always return IGT_MAX_PIP= ES */ > - for_each_pipe(display, p) num_pipes++; > - > - igt_skip_on_f(pipe > num_pipes, > - "ASIC does not have %d pipes\n", pipe); > - > test_init(data); > > /* create buffers */ > @@ -204,6 +196,7 @@ igt_main > { > data_t data; > int i =3D 0, j =3D 0; > + enum pipe p; > > memset(&data, 0, sizeof(data)); > > @@ -219,14 +212,13 @@ igt_main > > } > > - /* We're not using for_each_pipe_static because we need the > - * _amount_ of pipes */ > - for (i =3D 0; i < IGT_MAX_PIPES; i++) { > + for_each_pipe(&data.display, p) { > for (j =3D 0; j < ARRAY_SIZE(test_mode); j++) { > igt_subtest_f("linear-tiling-%d-displays-%s", i+1, > test_mode[j].name) > run_test_linear_tiling(&data, i, &test_mode[j]); > } > + i++; > } When enumerating tests with --list-subtests you don't have data.display valid. As gitlab pipeline relay warned for this series. What you can do is make this an igt_subtest_with_dynamic, and igt_subtest_with_dynamic_f("linear-tiling-multiple-displays-%s", test_mode[= j].name) { for_each_pipe(&data.display, p) { igt_dynamic_f("%d-displays", i+1) { run_test_linear_tiling(&data, i, &test_mode[j]); } } } However, even with that in place, using for_each_pipe sounds wrong. As the comment says, this is using an _amount_ of pipes, not one particular pipe. -- Petri Latvala --_000_BYAPR12MB3048BC33EEB684DC1A14DFD0F7E09BYAPR12MB3048namp_ Content-Type: text/html; charset="us-ascii" Content-Transfer-Encoding: quoted-printable

[AMD Official Use Only - General]


Thanks for the feedback. I will drop patch 2
From: Petri Latvala <pet= ri.latvala@intel.com>
Sent: 14 December 2022 01:30
To: Hung, Alex <Alex.Hung@amd.com>
Cc: igt-dev@lists.freedesktop.org <igt-dev@lists.freedesktop.org&= gt;
Subject: Re: [igt-dev] [PATCH 2/2] tests/kms_bw: Run tests on availa= ble pipes only
 
On Tue, Dec 13, 2022 at 04:43:32PM -0700, Alex Hun= g wrote:
> This cleans up unnecessary skips during executions.
>
> Signed-off-by: Alex Hung <alex.hung@amd.com>
> ---
>  tests/kms_bw.c | 16 ++++------------
>  1 file changed, 4 insertions(+), 12 deletions(-)
>
> diff --git a/tests/kms_bw.c b/tests/kms_bw.c
> index 27100587..ac801746 100644
> --- a/tests/kms_bw.c
> +++ b/tests/kms_bw.c
> @@ -145,17 +145,9 @@ static void run_test_linear_tiling(data_t *data, = int pipe, const drmModeModeInfo
>        igt_output_t *output;
>        struct igt_fb buffer[IGT_MAX= _PIPES];
>        igt_crc_t zero, captured[IGT= _MAX_PIPES];
> -     int i =3D 0, num_pipes =3D 0;
> -     enum pipe p;
> +     int i =3D 0;
>        int ret;
> 
> -     /* Cannot use igt_display_get_n_pipes() due = to fused pipes on i915 where they do
> -      * not give the numver of valid crtcs a= nd always return IGT_MAX_PIPES */
> -     for_each_pipe(display, p) num_pipes++;
> -
> -     igt_skip_on_f(pipe > num_pipes,
> -           &nb= sp;          "ASIC does n= ot have %d pipes\n", pipe);
> -
>        test_init(data);
> 
>        /* create buffers */
> @@ -204,6 +196,7 @@ igt_main
>  {
>        data_t data;
>        int i =3D 0, j =3D 0;
> +     enum pipe p;
> 
>        memset(&data, 0, sizeof(= data));
> 
> @@ -219,14 +212,13 @@ igt_main
> 
>        }
> 
> -     /* We're not using for_each_pipe_static beca= use we need the
> -      * _amount_ of pipes */
> -     for (i =3D 0; i < IGT_MAX_PIPES; i++) { > +     for_each_pipe(&data.display, p) {
>            = ;    for (j =3D 0; j < ARRAY_SIZE(test_mode); j++) {
>            = ;            igt_sub= test_f("linear-tiling-%d-displays-%s", i+1,
>            = ;            &n= bsp;     test_mode[j].name)
>            = ;            run_tes= t_linear_tiling(&data, i, &test_mode[j]);
>            = ;    }
> +           &nb= sp; i++;
>        }


When enumerating tests with --list-subtests you don't have
data.display valid. As gitlab pipeline relay warned for this series.

What you can do is make this an igt_subtest_with_dynamic, and

igt_subtest_with_dynamic_f("linear-tiling-multiple-displays-%s", = test_mode[j].name) {
  for_each_pipe(&data.display, p) {
    igt_dynamic_f("%d-displays", i+1) {
      run_test_linear_tiling(&data, i, &te= st_mode[j]);
    }
  }
}

However, even with that in place, using for_each_pipe sounds wrong. As
the comment says, this is using an _amount_ of pipes, not one
particular pipe.


--
Petri Latvala
--_000_BYAPR12MB3048BC33EEB684DC1A14DFD0F7E09BYAPR12MB3048namp_--