Re: [PATCH i-g-t v3 01/11] lib/i915/fbc: extract intel_fbc_get_fbc_status()

["Reddy Guddati, Santhosh" <santhosh.reddy.guddati@intel.com>]

Hi Vinod,

On 16-04-2026 14:55, Vinod Govindapillai wrote:
> Code to read the FBC status from the debugfs is being duplicated
> in many places. Extract intel_fbc_get_fbc_status() to be used as
> the single source to read the current FBC status.
> 
> Signed-off-by: Vinod Govindapillai <vinod.govindapillai@intel.com>
> ---
>   lib/i915/intel_fbc.c | 55 ++++++++++++++++++++++++++++++++++++--------
>   lib/i915/intel_fbc.h |  3 +++
>   2 files changed, 48 insertions(+), 10 deletions(-)
> 
> diff --git a/lib/i915/intel_fbc.c b/lib/i915/intel_fbc.c
> index b8c714d43..47f0e2fc1 100644
> --- a/lib/i915/intel_fbc.c
> +++ b/lib/i915/intel_fbc.c
> @@ -22,6 +22,47 @@ void intel_fbc_disable(igt_display_t *display)
>   	igt_set_module_param_int(display->drm_fd, "enable_fbc", 0);
>   }
>   
> +/**
> + * intel_fbc_get_fbc_status_crtc_index
> + * @device: fd of the device
> + * @crtc: crtc index
> + * @fbc_status: Buffer to fill the current fbc status
> + * @buf_size: Size of the buffer to be filled
> + *
> + * Read the debugfs entry for current fbc status of a crtc
> + *
> + * Returns:
> + * None
> + */
> +void intel_fbc_get_fbc_status_crtc_index(int device, int crtc_index,
> +					 char *fbc_status, int buf_size)
   The function name has redundant "fbc". Please consider renaming to 
intel_get_fbc_status_crtc_index?> +{
> +	int dir;
> +
> +	dir = igt_debugfs_crtc_dir(device, crtc_index);
> +	igt_require_fd(dir);
> +	igt_debugfs_simple_read(dir, "i915_fbc_status", fbc_status, buf_size);
> +	close(dir);
> +}
> +
> +/**
> + * intel_fbc_get_fbc_status
> + * @crtc: CRTC
> + * @fbc_status: Buffer to fill the current fbc status
> + * @buf_size: Size of the buffer to be filled
> + *
> + * Read the debugfs entry for current fbc status of a crtc
> + *
> + * Returns:
> + * None
> + */
> +void intel_fbc_get_fbc_status(igt_crtc_t *crtc, char *fbc_status, int buf_size)
> +{
> +	intel_fbc_get_fbc_status_crtc_index(crtc->display->drm_fd,
> +					    crtc->crtc_index, fbc_status,
> +					    buf_size);
> +}
> +
>   /**
>    * intel_fbc_supported:
>    * @crtc: CRTC
> @@ -34,12 +75,9 @@ void intel_fbc_disable(igt_display_t *display)
>   bool intel_fbc_supported(igt_crtc_t *crtc)
>   {
>   	char buf[FBC_STATUS_BUF_LEN];
> -	int dir;
>   
> -	dir = igt_crtc_debugfs_dir(crtc);
> -	igt_require_fd(dir);
> -	igt_debugfs_simple_read(dir, "i915_fbc_status", buf, sizeof(buf));
> -	close(dir);
> +	intel_fbc_get_fbc_status(crtc, buf, sizeof(buf));
> +
>   	if (*buf == '\0')
>   		return false;
>   
> @@ -51,12 +89,9 @@ static bool _intel_fbc_is_enabled(igt_crtc_t *crtc, int log_level, char *last_fb
>   {
>   	char buf[FBC_STATUS_BUF_LEN];
>   	bool print = true;
> -	int dir;
>   
> -	dir = igt_crtc_debugfs_dir(crtc);
> -	igt_require_fd(dir);
> -	igt_debugfs_simple_read(dir, "i915_fbc_status", buf, sizeof(buf));
> -	close(dir);
> +	intel_fbc_get_fbc_status(crtc, buf, sizeof(buf));
> +
>   	if (log_level != IGT_LOG_DEBUG)
>   		last_fbc_buf[0] = '\0';
>   	else if (strcmp(last_fbc_buf, buf))
> diff --git a/lib/i915/intel_fbc.h b/lib/i915/intel_fbc.h
> index 8e2662824..f8a506f23 100644
> --- a/lib/i915/intel_fbc.h
> +++ b/lib/i915/intel_fbc.h
> @@ -17,5 +17,8 @@ bool intel_fbc_wait_until_enabled(igt_crtc_t *crtc);
>   bool intel_fbc_is_enabled(igt_crtc_t *crtc, int log_level);
>   bool intel_fbc_plane_size_supported(igt_display_t *display, uint32_t width, uint32_t height);
>   bool intel_fbc_supported_for_psr_mode(igt_display_t *display, enum psr_mode mode);
> +void intel_fbc_get_fbc_status_crtc_index(int device, int crtc_index,
> +					 char *fbc_status, int buf_size);
> +void intel_fbc_get_fbc_status(igt_crtc_t *crtc, char *fbc_status, int buf_size);
>   
>   #endif