[Mesa-dev] [PATCH 1/2] i965: Rework the modifier info map

[Daniel Stone]

Hi,

On 17 July 2017 at 01:48, Jason Ekstrand <jason at jlekstrand.net> wrote:
> This commit splits the mapping in half.  The modifier_infos table now
> only contains the modifier and the since_gen field.  The tiling bits
> have been moved into a table in modifier_to_tiling as that's the only
> place it was ever used.  The modifier_is_supported function now takes a
> devinfo and does the since_gen check.

Any reason to not just drop it the modifier <-> tiling map completely
and use isl_drm_modifier_get_info() + isl_tiling_to_i915_tiling()? I'm
all for more of this code being deleted! :)

>  static const struct {
> -   uint32_t tiling;
>     uint64_t modifier;
>     unsigned since_gen;
> -} tiling_modifier_map[] = {
> -   { .tiling = I915_TILING_NONE, .modifier = DRM_FORMAT_MOD_LINEAR,
> -     .since_gen = 1 },
> -   { .tiling = I915_TILING_X, .modifier = I915_FORMAT_MOD_X_TILED,
> -     .since_gen = 1 },
> -   { .tiling = I915_TILING_Y, .modifier = I915_FORMAT_MOD_Y_TILED,
> -     .since_gen = 6 },
> +} supported_modifiers[] = {
> +   { DRM_FORMAT_MOD_LINEAR       , 1 },
> +   { I915_FORMAT_MOD_X_TILED     , 1 },
> +   { I915_FORMAT_MOD_Y_TILED     , 6 },
>  };

Losing named initialisation makes me very sad. Is there any
buildsys/compiler reason to do this?

The rest LGTM, so with or without ISL:
Reviewed-by: Daniel Stone <daniels at collabora.com>

Cheers,
Daniel