Re: [igt-dev] [i-g-t 5/5] tests/intel/kms_dirtyfb: Restructure dynamic subtests

["Hogander, Jouni" <jouni.hogander@intel.com>]

On Wed, 2023-11-15 at 12:33 +0530, Bhanuprakash Modem wrote:
> Instead of having all features under single subtest, create
> a subtest for each feature.
> 
> Before:
> igt@kms_dirtyfb@dirtyfb-ioctl@psr-eDP-1
> igt@kms_dirtyfb@dirtyfb-ioctl@fbc-eDP-1
> igt@kms_dirtyfb@dirtyfb-ioctl@drrs-eDP-1
> 
> After:
> igt@kms_dirtyfb@psr-dirtyfb-ioctl@pipe-A-eDP-1
> igt@kms_dirtyfb@fbc-dirtyfb-ioctl@pipe-A-eDP-1
> igt@kms_dirtyfb@drrs-dirtyfb-ioctl@pipe-A-eDP-1

Obviously you have some specific reason for this change:

Reviewed-by: Jouni Högander <jouni.hogander@intel.com>

Just for curiosity, what is the problem with the original approach?

BR,

Jouni Högander
> 
> Cc: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
> Cc: Jouni Högander <jouni.hogander@intel.com>
> Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
> ---
>  tests/intel/kms_dirtyfb.c | 67 +++++++++++++++++++++++++++----------
> --
>  1 file changed, 46 insertions(+), 21 deletions(-)
> 
> diff --git a/tests/intel/kms_dirtyfb.c b/tests/intel/kms_dirtyfb.c
> index a0f61b7a3..5ab2dc59f 100644
> --- a/tests/intel/kms_dirtyfb.c
> +++ b/tests/intel/kms_dirtyfb.c
> @@ -20,15 +20,26 @@ IGT_TEST_DESCRIPTION("Test the DIRTYFB ioctl is
> working properly with "
>   * TEST: kms dirtyfb
>   * Category: Display
>   * Description: Test DIRTYFB ioctl functionality.
> + * Driver requirement: i915, xe
> + * Functionality: dirtyfb
> + * Mega feature: General Display Features
> + * Test category: functionality test
>   *
> - * SUBTEST: dirtyfb-ioctl
> + * SUBTEST: default-dirtyfb-ioctl
>   * Description: Test DIRTYFB ioctl is working properly using GPU
>   *              frontbuffer rendering with features like FBC, PSR
>   *              and DRRS.
> - * Driver requirement: i915, xe
> - * Functionality: dirtyfb, fbc, psr, drrs
> - * Mega feature: General Display Features
> - * Test category: functionality test
> + *
> + * SUBTEST: %s-dirtyfb-ioctl
> + * Description: Test DIRTYFB ioctl is working properly using GPU
> + *              frontbuffer rendering with %arg[1] feature.
> + * Functionality: dirtyfb, %arg[1]
> + *
> + * arg[1]:
> + *
> + * @drrs:    drrs
> + * @fbc:     fbc
> + * @psr:     psr1
>   */
>  
>  #ifndef PAGE_ALIGN
> @@ -160,10 +171,6 @@ static void prepare(data_t *data)
>  {
>         igt_plane_t *primary;
>  
> -       igt_skip_on(!check_support(data));
> -
> -       igt_display_reset(&data->display);
> -
>         data->mode = igt_output_get_mode(data->output);
>  
>         igt_output_set_pipe(data->output, data->pipe);
> @@ -297,19 +304,37 @@ igt_main
>                 igt_display_reset(&data.display);
>         }
>  
> -       igt_describe("Test dirtyFB ioctl");
> -       igt_subtest_with_dynamic("dirtyfb-ioctl") {
> -               data.pipe = PIPE_A;
> -               for_each_valid_output_on_pipe(&data.display,
> data.pipe,
> -                                             data.output) {
> -                       for (data.feature = FEATURE_DEFAULT;
> data.feature > 0;
> -                            data.feature = data.feature >> 1) {
> -                               igt_dynamic_f("%s-%s",
> feature_str(data.feature),
> -                                            
> igt_output_name(data.output)) {
> -                                       prepare(&data);
> -                                       run_test(&data);
> -                                       cleanup(&data);
> +       for (data.feature = FEATURE_DEFAULT; data.feature > 0;
> +            data.feature = data.feature >> 1) {
> +               igt_describe_f("Test dirtyFB ioctl with %s",
> feature_str(data.feature));
> +               igt_subtest_with_dynamic_f("%s-dirtyfb-ioctl",
> feature_str(data.feature)) {
> +                       for_each_pipe(&data.display, data.pipe) {
> +                               int valid_tests = 0;
> +
> +                               for_each_valid_output_on_pipe(&data.d
> isplay,
> +                                                            
> data.pipe,
> +                                                            
> data.output) {
> +                                       if (!check_support(&data))
> +                                               continue;
> +
> +                                       igt_display_reset(&data.displ
> ay);
> +                                       igt_output_set_pipe(data.outp
> ut, data.pipe);
> +                                       if
> (!intel_pipe_output_combo_valid(&data.display))
> +                                               continue;
> +
> +                                       valid_tests++;
> +                                       igt_dynamic_f("%s-%s",
> +                                                    
> kmstest_pipe_name(data.pipe),
> +                                                    
> igt_output_name(data.output)) {
> +                                               prepare(&data);
> +                                               run_test(&data);
> +                                               cleanup(&data);
> +                                       }
>                                 }
> +
> +                               /* One pipe is enough. */
> +                               if (valid_tests)
> +                                       break;
>                         }
>                 }
>         }