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

[Jason Ekstrand]

On Mon, Jul 17, 2017 at 2:24 AM, Daniel Stone <daniel at fooishbar.org> wrote:

> 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! :)
>

Because the commit message is wrong.  It's the tiling_to_modifier function
into which it gets moved. :-)


> >  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?
>

Just because it made it one line per modifier.  That said, I think I could
probably put them back in.


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

Thanks!
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170717/b82de4ec/attachment.html>