[v4,2/7] lib/i915/intel_fbc: Add fbc supported check based on wa

[Kamil Konieczny]

On 2025-06-27 at 11:34:41 +0530, Mohammed Thasleem wrote:
> This will check the fbc supported based on given workarunds.
> 
> v2: Add intel_wa header file.
> v3: Add xe check. (Kamil)
> v4: Updated function name with intel_is_fbc_disabled_by_wa. (Kamil)

Please see comments below.

> 
> Signed-off-by: Mohammed Thasleem <mohammed.thasleem at intel.com>
> Reviewed-by: Kunal Joshi <kunal1.joshi at intel.com>
> ---
>  lib/i915/intel_fbc.c | 25 +++++++++++++++++++++++++
>  lib/i915/intel_fbc.h |  1 +
>  2 files changed, 26 insertions(+)
> 
> diff --git a/lib/i915/intel_fbc.c b/lib/i915/intel_fbc.c
> index a8ded0a59..722899432 100644
> --- a/lib/i915/intel_fbc.c
> +++ b/lib/i915/intel_fbc.c
> @@ -9,6 +9,7 @@
>  #include "igt_psr.h"
>  
>  #include "intel_fbc.h"
> +#include "intel_wa.h"
>  
>  #define FBC_STATUS_BUF_LEN 128
>  
> @@ -200,3 +201,27 @@ bool intel_fbc_supported_for_psr_mode(int disp_ver, enum psr_mode mode)
>  
>  	return fbc_supported;
>  }
> +
> +/**
> + * intel_is_fbc_disabled_by_wa
> + *
> + * @fd: fd of the device
> + *
> + * This function pass the WA to igt_has_intel_wa to check WA on available GTs

imho this should be:

 * This function check if WA is present on some GT, which in turn make
 * FBC not possible

> + *
> + * Returns:
> + * true if wa is not equal to 1, and false if wa is equal to 1

imho this should be reversed, also each return value in its own line,
so better:

 * true:  if WA is applied and FBC id disabled
 * false: otherwise

> + * true if wa is not equal to 1, and false if wa is equal to 1

Also, please do not translate C source code into english.

> + */
> +bool intel_is_fbc_disabled_by_wa(int fd)
> +{
> +	int wa;
> +	const char *wa_fbc_disabled = "16023588340";
> +
> +	if (!is_xe_device(fd))
> +		return true;

Should be:

		return false;

> +
> +	wa = igt_has_intel_wa(fd, wa_fbc_disabled);
> +	igt_assert_f(wa >= 0, "WA path not found on GTs\n");

Do you really need this assert here?

> +
> +	return wa != 1;

Should be:

	return wa == 1;

Update also caller sites into:

igt_skip_on_f(data.feature == FEATURE_FBC && intel_is_fbc_disabled_by_wa(data.drm_fd),
	      "WA has disabled FBC on BMG\n");

Also align to first char after '('.

Regards,
Kamil

> +}
> diff --git a/lib/i915/intel_fbc.h b/lib/i915/intel_fbc.h
> index fcafe6049..fe9edcbc3 100644
> --- a/lib/i915/intel_fbc.h
> +++ b/lib/i915/intel_fbc.h
> @@ -19,5 +19,6 @@ bool intel_fbc_is_enabled(int device, enum pipe pipe, int log_level);
>  void intel_fbc_max_plane_size(int fd, uint32_t *width, uint32_t *height);
>  bool intel_fbc_plane_size_supported(int device, uint32_t width, uint32_t height);
>  bool intel_fbc_supported_for_psr_mode(int disp_ver, enum psr_mode mode);
> +bool intel_is_fbc_disabled_by_wa(int fd);
>  
>  #endif