On Mon, Jul 17, 2017 at 2:24 AM, Daniel Stone <dan...@fooishbar.org> wrote:
> Hi,
>
> On 17 July 2017 at 01:48, Jason Ekstrand <ja...@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 <dani...@collabora.com>
>
Thanks!
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
