[igt-dev] [PATCH i-g-t 01/25] lib/igt_kms: Fill the plane format/mod information for pre-blobifier drivers

[Paulo Zanoni]

Em Qui, 2018-07-19 às 18:03 +0300, Ville Syrjala escreveu:
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> 
> For drivers that don't support the IN_FORMATS blob we should just
> consult the format list returned by getplane. Since we can't know
> which modifiers are supported we'll assume linear-only. Obviously
> that may not work for every driver out there, but not much more
> we can do unless we start to actually probing with addfb.

Can you please elaborate on why we need that change? What exactly do we
gain from it? IMHO there's always the worry that this could be misused
in the future, restricting tests to linear only when other formats are
indeed possible. Having NULL buffers crashing the test is a good way to
prevent misuse.

What if we populate plane->formats like we do here but leave plane-
>modifiers NULL? It would allow other pieces of the code to rely on the
format list while not exposing the risk of the "linear only" incorrect
restriction.

That said, the patch is correct in the sense that it does what it
proposes to do.

> 
> Cc: Ulrich Hecht <ulrich.hecht+renesas at gmail.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> ---
>  lib/igt_kms.c | 22 +++++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 476a786233c0..5641d8c1cf7c 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -4068,8 +4068,28 @@ static void
> igt_fill_plane_format_mod(igt_display_t *display, igt_plane_t *plane
>  	int idx = 0;
>  	int count;
>  
> -	if (!igt_plane_has_prop(plane, IGT_PLANE_IN_FORMATS))
> +	if (!igt_plane_has_prop(plane, IGT_PLANE_IN_FORMATS)) {
> +		drmModePlanePtr p = plane->drm_plane;
> +
> +		count = p->count_formats;
> +
> +		plane->format_mod_count = count;
> +		plane->formats = calloc(count, sizeof(plane-
> >formats[0]));
> +		igt_assert(plane->formats);
> +		plane->modifiers = calloc(count, sizeof(plane-
> >modifiers[0]));
> +		igt_assert(plane->modifiers);
> +
> +		/*
> +		 * We don't know which modifiers are
> +		 * supported, so we'll assume linear only.
> +		 */
> +		for (int i = 0; i < count; i++) {
> +			plane->formats[i] = p->formats[i];
> +			plane->modifiers[i] = DRM_FORMAT_MOD_LINEAR;
> +		}
> +
>  		return;
> +	}
>  
>  	blob_id = igt_plane_get_prop(plane, IGT_PLANE_IN_FORMATS);
>