[PATCH i-g-t] tests/intel/kms_frontbuffer_tracking: Refine FBC test conditions for BMG variants

[Kamil Konieczny]

Hi Mohammed,
On 2025-05-07 at 13:00:15 +0530, Mohammed Thasleem wrote:
> This update stops skipping fbc-* tests based on the BMG platform.
> Instead, tests are excluded only for specific unsupported BMG variants.
> 
> Signed-off-by: Mohammed Thasleem <mohammed.thasleem at intel.com>
> ---
>  lib/igt_kms.c                          | 30 ++++++++++++++++++++++++++
>  lib/igt_kms.h                          |  1 +
>  tests/intel/kms_frontbuffer_tracking.c |  7 +++---
>  3 files changed, 35 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index f3bc481f2..668da5915 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -61,6 +61,7 @@
>  #include "igt_device.h"
>  #include "igt_sysfs.h"
>  #include "sw_sync.h"
> +#include "xe/xe_query.h"
>  #ifdef HAVE_CHAMELIUM
>  #include "igt_chamelium.h"
>  #endif
> @@ -7442,3 +7443,32 @@ int igt_backlight_write(int value, const char *fname, igt_backlight_context_t *c
>  
>  	return 0;
>  }
> +
> +/**
> + * igt_read_debugfs_wa:
> + * @drm_fd:	A drm file descriptor
> + * @check_wa:	Wa to be checked

Add here description when true will be returned, from implementation
I see it will return true if WA is present in one of GTs.

> + */
> +bool igt_read_debugfs_wa(int drm_fd, const char *check_wa)

As name of this function suggest it should be placed in lib/igt_debugfs.h

Also, as this is intel specific, imho it's name should be

bool igt_has_intel_wa(int drm_fd, const char *check_wa)

> +{
> +	char buf[4096];
> +	bool found = false;
> +	char name[PATH_MAX];
> +	unsigned int xe, debugfs_fd;
> +

Add here check that drm_fd is for intel driver, i915 or xe.

> +	debugfs_fd = igt_debugfs_dir(drm_fd);
> +
> +	xe_for_each_gt(drm_fd, xe) {
> +		sprintf(name, "gt%d/workarounds", xe);
> +		igt_require(igt_debugfs_exists(drm_fd, name, O_RDONLY));

Hmmm, please avoid igt_require, if you really need it add gt as param
and just return false if no 'workarounds' file present.

Or add second function for all GTs checks, not one of them, but then
why true when wa is present on one GT? Why not on all of them?

Regards,
Kamil

> +
> +		if (igt_debugfs_simple_read(debugfs_fd, name, buf, sizeof(buf)) > 0) {
> +			if (strstr(buf, check_wa)) {
> +				found = true;
> +				break;
> +			}
> +		}
> +	}
> +
> +	return found;
> +}
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index 0381c82ad..cd83a56e7 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -1279,5 +1279,6 @@ void igt_set_link_params(int drm_fd, igt_output_t *output,
>  			   char *link_rate, char *lane_count);
>  int igt_backlight_read(int *result, const char *fname, igt_backlight_context_t *context);
>  int igt_backlight_write(int value, const char *fname, igt_backlight_context_t *context);
> +bool igt_read_debugfs_wa(int drm_fd, const char *check_wa);
>  
>  #endif /* __IGT_KMS_H__ */
> diff --git a/tests/intel/kms_frontbuffer_tracking.c b/tests/intel/kms_frontbuffer_tracking.c
> index 0809352cb..56d6c6f74 100644
> --- a/tests/intel/kms_frontbuffer_tracking.c
> +++ b/tests/intel/kms_frontbuffer_tracking.c
> @@ -781,6 +781,7 @@
>   */
>  
>  #define TIME SLOW_QUICK(1000, 10000)
> +#define CHECK_WA "16023588340"
>  
>  IGT_TEST_DESCRIPTION("Test the Kernel's frontbuffer tracking mechanism and "
>  		     "its related features: FBC, PSR and DRRS");
> @@ -2610,7 +2611,7 @@ static void prepare_subtest_data(const struct test_mode *t,
>  static void prepare_subtest_screens(const struct test_mode *t)
>  {
>  	/* FBC disabled: Wa_16023588340 */
> -	igt_skip_on_f((IS_BATTLEMAGE(drm.devid) && t->feature == FEATURE_FBC),
> +	igt_skip_on_f((igt_read_debugfs_wa(drm.fd, CHECK_WA) && t->feature == FEATURE_FBC),
>  		      "FBC isn't supported on BMG\n");
>  
>  	if (t->pipes == PIPE_DUAL)
> @@ -2658,7 +2659,7 @@ static void prepare_subtest(const struct test_mode *t,
>  static void rte_subtest(const struct test_mode *t)
>  {
>  	/* FBC disabled: Wa_16023588340 */
> -	igt_skip_on_f((IS_BATTLEMAGE(drm.devid) && t->feature == FEATURE_FBC),
> +	igt_skip_on_f((igt_read_debugfs_wa(drm.fd, CHECK_WA) && t->feature == FEATURE_FBC),
>  		      "FBC isn't supported on BMG\n");
>  
>  	prepare_subtest_data(t, NULL);
> @@ -4157,7 +4158,7 @@ igt_main_args("", long_options, help_str, opt_handler, NULL)
>  			t.tiling = opt.tiling;
>  
>  			/* FBC disabled: Wa_16023588340 */
> -			igt_skip_on_f((IS_BATTLEMAGE(drm.devid) && t.feature == FEATURE_FBC),
> +			igt_skip_on_f((igt_read_debugfs_wa(drm.fd, CHECK_WA) && t.feature == FEATURE_FBC),
>  				      "FBC isn't supported on BMG\n");
>  
>  			for_each_pipe(&drm.display, pipe) {
> -- 
> 2.34.1
>