[igt-dev] [PATCH i-g-t 02/25] tests/kms_ccs: Use igt_plane_has_format_mod()

[Paulo Zanoni]

Em Qui, 2018-07-19 às 18:03 +0300, Ville Syrjala escreveu:
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> 
> Replace the open coded IN_FORMATS parsing with
> igt_plane_has_format_mod() now that we have such a thing.

This patch removes a few of the igt_skip() and igt_assert() calls we
had. Previously explicit messages such as "IN_FORMATS not supported by
X" will become "requirement valid_tests == 0 failed", which IMHO is not
exactly an improvement to the code base.

Can you please keep the first kmstest_get_property() and the two
igt_skip_on() calls that follow it?

If we keep the assertions:
Reviewed-by: Paulo Zanoni <paulo.r.zanoni at intel.com>


> 
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> ---
>  tests/kms_ccs.c | 50 +--------------------------------------------
> -----
>  1 file changed, 1 insertion(+), 49 deletions(-)
> 
> diff --git a/tests/kms_ccs.c b/tests/kms_ccs.c
> index e1ee58801ac3..fe21b3a866de 100644
> --- a/tests/kms_ccs.c
> +++ b/tests/kms_ccs.c
> @@ -132,55 +132,7 @@ modifiers_ptr(struct
> local_drm_format_modifier_blob *blob)
>  
>  static bool plane_has_format_with_ccs(data_t *data, igt_plane_t
> *plane, uint32_t format)
>  {
> -	drmModePropertyBlobPtr blob;
> -	struct local_drm_format_modifier_blob *blob_data;
> -	struct local_drm_format_modifier *modifiers, *last_mod;
> -	uint32_t *formats, *last_fmt;
> -	uint64_t blob_id;
> -	bool ret;
> -	int fmt_idx = -1;
> -
> -	ret = kmstest_get_property(data->drm_fd, plane->drm_plane-
> >plane_id,
> -				   DRM_MODE_OBJECT_PLANE,
> "IN_FORMATS",
> -				   NULL, &blob_id, NULL);
> -	igt_skip_on_f(ret == false, "IN_FORMATS not supported by
> kernel\n");
> -	igt_skip_on_f(blob_id == 0, "IN_FORMATS not supported by
> plane\n");
> -	blob = drmModeGetPropertyBlob(data->drm_fd, blob_id);
> -	igt_assert(blob);
> -	igt_assert_lte(sizeof(struct
> local_drm_format_modifier_blob),
> -		       blob->length);
> -
> -	blob_data = (struct local_drm_format_modifier_blob *) blob-
> >data;
> -	formats = formats_ptr(blob_data);
> -	last_fmt = &formats[blob_data->count_formats];
> -	igt_assert_lte(((char *) last_fmt - (char *) blob_data),
> blob->length);
> -	for (int i = 0; i < blob_data->count_formats; i++) {
> -		if (formats[i] == format) {
> -			fmt_idx = i;
> -			break;
> -		}
> -	}
> -
> -	if (fmt_idx == -1)
> -		return false;
> -
> -	modifiers = modifiers_ptr(blob_data);
> -	last_mod = &modifiers[blob_data->count_modifiers];
> -	igt_assert_lte(((char *) last_mod - (char *) blob_data),
> blob->length);
> -	for (int i = 0; i < blob_data->count_modifiers; i++) {
> -		if (modifiers[i].modifier !=
> LOCAL_I915_FORMAT_MOD_Y_TILED_CCS)
> -			continue;
> -
> -		if (modifiers[i].offset > fmt_idx ||
> -		    fmt_idx > modifiers[i].offset + 63)
> -			continue;
> -
> -		if (modifiers[i].formats &
> -		    (1UL << (fmt_idx - modifiers[i].offset)))
> -			return true;
> -	}
> -
> -	return false;
> +	return igt_plane_has_format_mod(plane, format,
> LOCAL_I915_FORMAT_MOD_Y_TILED_CCS);
>  }
>  
>  static void render_fb(data_t *data, uint32_t gem_handle, unsigned
> int size,