[PATCH i-g-t v4 7/8] XE3: tests/intel/kms_frontbuffer_tracking: fix x-tiled tests for case when there is no x-tile

[Matt Roper]

On Fri, Dec 06, 2024 at 01:27:47PM -0800, Clint Taylor wrote:
> From: "Heikkila, Juha-pekka" <juha-pekka.heikkila at intel.com>
> 
> On Xe3 display no more support x-tile and will disable such framebuffers
> 
> v2: Optimize display_ver checks (Sai Teja)
> 
> Signed-off-by: Heikkila, Juha-pekka <juha-pekka.heikkila at intel.com>
> Signed-off-by: Clint Taylor <Clinton.A.Taylor at intel.com>
> ---
>  tests/intel/kms_frontbuffer_tracking.c | 25 +++++++++++++++++++++----
>  1 file changed, 21 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/intel/kms_frontbuffer_tracking.c b/tests/intel/kms_frontbuffer_tracking.c
> index e41ee0a80..c5b3fd46f 100644
> --- a/tests/intel/kms_frontbuffer_tracking.c
> +++ b/tests/intel/kms_frontbuffer_tracking.c
> @@ -2181,7 +2181,12 @@ static void setup_modeset(void)
>  	offscreen_fb.fb = NULL;
>  	offscreen_fb.w = 1024;
>  	offscreen_fb.h = 1024;
> -	create_fbs(FORMAT_DEFAULT, opt.tiling);
> +
> +	/* Xe3 remove x-tile from display + */
> +	if (drm.display_ver < 30)
> +		create_fbs(FORMAT_DEFAULT, opt.tiling);
> +	else
> +		create_fbs(FORMAT_DEFAULT, TILING_4);

We should really try to get away from hardcoding assumptions like this.
Real userspace can't check the display IP version and is expected to use
the information the kernel already advertises.  It would be better if
unspecified/default tiling just walks down a list of modifiers and
uses the IGT helper to check whether the modifier is supported; once it
finds one that's supported it can use that.  Tests that try to
explicitly use a specific tiling should skip if the kernel doesn't list
any modifiers with that tiling.  And things like this where the user can
manually specify a specific tiling format on the command line should
just plain fail if they specify a tiling type that isn't supported (we
shouldn't just silently ignore what they were asking for).

>  }
>  
>  static void teardown_modeset(void)
> @@ -2302,7 +2307,6 @@ static void setup_drrs(void)
>  
>  static void setup_environment(void)
>  {
> -	setup_drm();
>  	setup_modeset();
>  
>  	setup_fbc();
> @@ -3146,6 +3150,7 @@ static bool tiling_is_valid(int feature_flags, enum tiling_type tiling)
>  	case TILING_LINEAR:
>  		return intel_gen(drm.devid) >= 9;
>  	case TILING_X:
> +		return (intel_get_device_info(drm.devid)->display_ver > 29) ? false : true;

I don't think this is necessary.  This function is only used by the
"%s-tiling-%s" subtest, and it's already bailed out if tiling is X-tile
before we call this function.


>  	case TILING_Y:
>  		return true;
>  	case TILING_4:
> @@ -4226,9 +4231,10 @@ igt_main_args("", long_options, help_str, opt_handler, NULL)
>  	igt_output_t *output;
>  
>  	igt_fixture {
> -		setup_environment();
> +		setup_drm();
>  		drm.devid = intel_get_drm_devid(drm.fd);
>  		drm.display_ver = intel_display_ver(drm.devid);
> +		setup_environment();
>  	}
>  
>  	for (t.feature = 0; t.feature < FEATURE_COUNT; t.feature++) {
> @@ -4537,6 +4543,11 @@ igt_main_args("", long_options, help_str, opt_handler, NULL)
>  					if (t.tiling == TILING_4)
>  						igt_require(intel_get_device_info(drm.devid)->has_4tile);
>  
> +					/* Xe3 remove x-tile from display + */
> +					if (t.tiling == TILING_X) {
> +						igt_require(drm.display_ver < 30);
> +					}

This function already has a

    if (t.tiling == TILING_X)
            continue;

earlier, so we can never get here with X-tiling.


Matt

> +
>  					if (tiling_is_valid(t.feature, t.tiling))
>  						draw_subtest(&t);
>  					else
> @@ -4568,7 +4579,13 @@ igt_main_args("", long_options, help_str, opt_handler, NULL)
>  	t.format = FORMAT_DEFAULT;
>  	t.flip = FLIP_PAGEFLIP;
>  	t.method = IGT_DRAW_BLT;
> -	t.tiling = opt.tiling;
> +
> +	/* Xe3 remove x-tile from display + */
> +	if (drm.display_ver < 30)
> +		t.tiling = opt.tiling;
> +	else
> +		t.tiling = TILING_4;
> +
>  	igt_subtest("basic") {
>  		if (!is_xe_device(drm.fd))
>  			igt_require_gem(drm.fd);
> -- 
> 2.25.1
> 

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation