Re: [v4,2/2] tests/intel/kms_frontbuffer_tracking: Enable the tests support variations based on the WAs

["Joshi, Kunal1" <kunal1.joshi@intel.com>]

Hello Thasleem,

On 01-06-2025 18:13, Mohammed Thasleem wrote:
> This update stops skipping fbc-* tests support variations based on the WAs.
>
> v2: Use WA_FBC_DISABLED instead CHECK_WA. (Vinod)
>      Update igt_skip_on_f discription. (Vinod)
> v3: Remove platform check and make it for xe specific. (Vinod)
>      Rename function name with is_fbc_support. (Vinod)
> v4: Update function name and add igt_skip_on_f at calling function. (Vinod)
>
> Signed-off-by: Mohammed Thasleem <mohammed.thasleem@intel.com>
> Reviewed-by: Vinod Govindapillai <vinod.govindapillai@intel.com>
> ---
>   tests/intel/kms_frontbuffer_tracking.c | 23 +++++++++++++++++------
>   1 file changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/tests/intel/kms_frontbuffer_tracking.c b/tests/intel/kms_frontbuffer_tracking.c
> index 0809352cb..66a5177fe 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 WA_FBC_DISABLED "16023588340"
>   
>   IGT_TEST_DESCRIPTION("Test the Kernel's frontbuffer tracking mechanism and "
>   		     "its related features: FBC, PSR and DRRS");
> @@ -2248,6 +2249,16 @@ static void do_flush(const struct test_mode *t)
>   
>   #define ASSERT_NO_IDLE_GPU		(1 << 11)
>   
> +static bool is_fbc_supported(int devid, const struct test_mode *t)
> +{
> +	int wa;
> +
> +	wa = igt_has_intel_wa(drm.fd, WA_FBC_DISABLED);
> +	igt_assert_f(wa >= 0, "WA path not found on GTs\n");
> +
> +	return wa != 1;
> +}
Better to move this to a new helper (kms_fbc_helper.c) or some library
since kms_dirtyfb, kms_fbcon_fbt, kms_psr and kms_psr2_sf may require
similar check.

Apart from that LGTM, you can have my rb on both patch of the series.
Reviewed-by: Kunal Joshi <kunal1.joshi@intel.com>
> +
>   static int adjust_assertion_flags(const struct test_mode *t, int flags)
>   {
>   	if (!(flags & DONT_ASSERT_FEATURE_STATUS)) {
> @@ -2610,8 +2621,8 @@ 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),
> -		      "FBC isn't supported on BMG\n");
> +	igt_skip_on_f(t->feature == FEATURE_FBC && !is_fbc_supported(drm.devid, t),
> +		     "WA has disabled FBC on BMG\n");
>   
>   	if (t->pipes == PIPE_DUAL)
>   		enable_both_screens_and_wait(t);
> @@ -2658,8 +2669,8 @@ 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),
> -		      "FBC isn't supported on BMG\n");
> +	igt_skip_on_f(t->feature == FEATURE_FBC && !is_fbc_supported(drm.devid, t),
> +		     "WA has disabled FBC on BMG\n");
>   
>   	prepare_subtest_data(t, NULL);
>   
> @@ -4157,8 +4168,8 @@ 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),
> -				      "FBC isn't supported on BMG\n");
> +			igt_skip_on_f(t.feature == FEATURE_FBC && !is_fbc_supported(drm.devid, &t),
> +				     "WA has disabled FBC on BMG\n");
>   
>   			for_each_pipe(&drm.display, pipe) {
>   				if (pipe == default_pipe) {