[PATCH][V4] lib/igt_kms: Move get_writeback_formats_blob to lib

[Kamil Konieczny]

Hi Alex,
On 2025-06-25 at 11:41:15 -0600, Alex Hung wrote:
> From: Harry Wentland <harry.wentland at amd.com>
> 
> This function can be used by other tests that need writeback. A prefix
> "igt_" is also added to the function name.
> 
> Signed-off-by: Alex Hung <alex.hung at amd.com>
> Signed-off-by: Harry Wentland <harry.wentland at amd.com>
> ---
>  lib/igt_kms.c         | 25 +++++++++++++++++++++++++
>  lib/igt_kms.h         |  2 ++
>  tests/kms_writeback.c | 22 ++--------------------
>  3 files changed, 29 insertions(+), 20 deletions(-)
> 
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 9ee03e870..d71076f24 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -7472,3 +7472,28 @@ int igt_backlight_write(int value, const char *fname, igt_backlight_context_t *c
>  
>  	return 0;
>  }
> +
> +/**
> + * igt_get_writeback_formats_blob:
> + * @output: Target output
> + *
> + * get supported formats from the writeback connector
> + *
> + * Returns: pointer to the writeback formats blob or NULL if not available
> + */
> +drmModePropertyBlobRes *igt_get_writeback_formats_blob(igt_output_t *output)
> +{
> +	drmModePropertyBlobRes *blob = NULL;
> +	uint64_t blob_id;
> +	int ret;
> +
> +	ret = kmstest_get_property(output->display->drm_fd,
> +				   output->config.connector->connector_id,
> +				   DRM_MODE_OBJECT_CONNECTOR,
> +				   igt_connector_prop_names[IGT_CONNECTOR_WRITEBACK_PIXEL_FORMATS],
> +				   NULL, &blob_id, NULL);
> +	if (ret)
> +		blob = drmModeGetPropertyBlob(output->display->drm_fd, blob_id);
> +
> +	return blob;
> +}
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index a85fee515..09d7d4414 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -1281,4 +1281,6 @@ void igt_set_link_params(int drm_fd, igt_output_t *output,
>  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);
>  
> +drmModePropertyBlobRes *igt_get_writeback_formats_blob(igt_output_t *output);
> +
>  #endif /* __IGT_KMS_H__ */
> diff --git a/tests/kms_writeback.c b/tests/kms_writeback.c
> index 8765fda34..b4c3b1cd2 100644
> --- a/tests/kms_writeback.c
> +++ b/tests/kms_writeback.c
> @@ -92,25 +92,6 @@ enum {
>  	XRGB2101010 = 1 << 1,
>  };
>  
> -static drmModePropertyBlobRes *get_writeback_formats_blob(igt_output_t *output)
> -{
> -	drmModePropertyBlobRes *blob = NULL;
> -	uint64_t blob_id;
> -	int ret;
> -
> -	ret = kmstest_get_property(output->display->drm_fd,
> -				   output->config.connector->connector_id,
> -				   DRM_MODE_OBJECT_CONNECTOR,
> -				   igt_connector_prop_names[IGT_CONNECTOR_WRITEBACK_PIXEL_FORMATS],
> -				   NULL, &blob_id, NULL);
> -	if (ret)
> -		blob = drmModeGetPropertyBlob(output->display->drm_fd, blob_id);
> -
> -	igt_assert(blob);
> -
> -	return blob;
> -}
> -
>  static bool check_writeback_config(igt_display_t *display, igt_output_t *output,
>  				    drmModeModeInfo override_mode)
>  {
> @@ -621,7 +602,8 @@ igt_main_args("b:c:f:dl", long_options, help_str, opt_handler, NULL)
>  		const char *valid_chars;
>  
>  		igt_skip_on(data.dump_check || data.list_modes);
> -		formats_blob = get_writeback_formats_blob(output);
> +		formats_blob = igt_get_writeback_formats_blob(output);
> +		igt_require_f(formats_blob, "No writeback pixel formats\n");

Now imho it looks good but I am not KMS dev so I am not sure
if not having pixel formats is a bug or not, e.g. if we need
igt_assert_f here or igt_require_f is good enough.

Khartik or Swati or Juha-Pekka, could you help here?

+cc Karthik B S <karthik.b.s at intel.com>
Cc: Juha-Pekka Heikkila <juha-pekka.heikkila at intel.com>
Cc: Swati Sharma <swati2.sharma at intel.com>

So moving function to lib looks ok, for that part

Reviewed-by: Kamil Konieczny <kamil.konieczny at linux.intel.com>

>  		valid_chars = "01234568 ABCGNRUVXY";
>  
>  		/*
> -- 
> 2.43.0
>