Re: [PATCH i-g-t v4 8/8] XE3: tests/intel/kms_frontbuffer_tracking: platform based handling of x-tile cases

[Matt Roper <matthew.d.roper@intel.com>]

On Fri, Dec 06, 2024 at 01:27:48PM -0800, Clint Taylor wrote:
> From: Vinod Govindapillai <vinod.govindapillai@intel.com>
> 
> x-tile is not supported from xe3 onwards. Handle this conclusively
> in all the kms_frontbuffer_tracking subtest options.

This patch is undoing a bunch of the changes from the previous patch.
These need to be combined.

> 
> Signed-off-by: Vinod Govindapillai <vinod.govindapillai@intel.com>
> Signed-off-by: Clint Taylor <Clinton.A.Taylor@intel.com>
> ---
>  tests/intel/kms_frontbuffer_tracking.c | 29 +++++++++++---------------
>  1 file changed, 12 insertions(+), 17 deletions(-)
> 
> diff --git a/tests/intel/kms_frontbuffer_tracking.c b/tests/intel/kms_frontbuffer_tracking.c
> index c5b3fd46f..ba564fafa 100644
> --- a/tests/intel/kms_frontbuffer_tracking.c
> +++ b/tests/intel/kms_frontbuffer_tracking.c
> @@ -1031,7 +1031,7 @@ struct test_mode {
>  		TILING_Y,
>  		TILING_4,
>  		TILING_COUNT,
> -		TILING_DEFAULT = TILING_X,
> +		TILING_AUTOSELECT,
>  	} tiling;
>  
>  	enum igt_draw_method method;
> @@ -1154,7 +1154,7 @@ struct {
>  	.only_pipes = PIPE_COUNT,
>  	.shared_fb_x_offset = 248,
>  	.shared_fb_y_offset = 500,
> -	.tiling = TILING_DEFAULT,
> +	.tiling = TILING_AUTOSELECT,
>  };
>  
>  struct modeset_params {
> @@ -2182,11 +2182,7 @@ static void setup_modeset(void)
>  	offscreen_fb.w = 1024;
>  	offscreen_fb.h = 1024;
>  
> -	/* Xe3 remove x-tile from display + */
> -	if (drm.display_ver < 30)
> -		create_fbs(FORMAT_DEFAULT, opt.tiling);
> -	else
> -		create_fbs(FORMAT_DEFAULT, TILING_4);
> +	create_fbs(FORMAT_DEFAULT, opt.tiling);
>  }
>  
>  static void teardown_modeset(void)
> @@ -4234,6 +4230,14 @@ igt_main_args("", long_options, help_str, opt_handler, NULL)
>  		setup_drm();
>  		drm.devid = intel_get_drm_devid(drm.fd);
>  		drm.display_ver = intel_display_ver(drm.devid);
> +
> +		/* TILING_X is not supported from Xe3 onwards. If the tiling
> +		 * is not set explicitly using the commandline parameter,
> +		 * handle the default tiling based on the platform.
> +		 */
> +		if (opt.tiling == TILING_AUTOSELECT)
> +			opt.tiling = drm.display_ver >= 30 ? TILING_4 : TILING_X;

As noted on the previous patch, I think we should try to avoid looking
at display_ver (since this isn't proper userspace behavior).  Instead we
should have a list of tiling formats (x-tile, 4-tile, y-tile, linear,
etc.) and we should just walk down that list and test each one with
igt_display_has_format_mod() until we find one the device supports; that
first match can be the 'autoselect' value.


Matt

> +
>  		setup_environment();
>  	}
>  
> @@ -4543,10 +4547,6 @@ 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);
> -					}
>  
>  					if (tiling_is_valid(t.feature, t.tiling))
>  						draw_subtest(&t);
> @@ -4579,12 +4579,7 @@ igt_main_args("", long_options, help_str, opt_handler, NULL)
>  	t.format = FORMAT_DEFAULT;
>  	t.flip = FLIP_PAGEFLIP;
>  	t.method = IGT_DRAW_BLT;
> -
> -	/* Xe3 remove x-tile from display + */
> -	if (drm.display_ver < 30)
> -		t.tiling = opt.tiling;
> -	else
> -		t.tiling = TILING_4;
> +	t.tiling = opt.tiling;
>  
>  	igt_subtest("basic") {
>  		if (!is_xe_device(drm.fd))
> -- 
> 2.25.1
> 

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