<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Jul 17, 2017 at 2:24 AM, Daniel Stone <span dir="ltr"><<a href="mailto:daniel@fooishbar.org" target="_blank">daniel@fooishbar.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi,<br>
<span class=""><br>
On 17 July 2017 at 01:48, Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> wrote:<br>
> This commit splits the mapping in half.  The modifier_infos table now<br>
> only contains the modifier and the since_gen field.  The tiling bits<br>
> have been moved into a table in modifier_to_tiling as that's the only<br>
> place it was ever used.  The modifier_is_supported function now takes a<br>
> devinfo and does the since_gen check.<br>
<br>
</span>Any reason to not just drop it the modifier <-> tiling map completely<br>
and use isl_drm_modifier_get_info() + isl_tiling_to_i915_tiling()? I'm<br>
all for more of this code being deleted! :)<span class=""><br></span></blockquote><div><br></div><div>Because the commit message is wrong.  It's the tiling_to_modifier function into which it gets moved. :-)<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
>  static const struct {<br>
> -   uint32_t tiling;<br>
>     uint64_t modifier;<br>
>     unsigned since_gen;<br>
> -} tiling_modifier_map[] = {<br>
> -   { .tiling = I915_TILING_NONE, .modifier = DRM_FORMAT_MOD_LINEAR,<br>
> -     .since_gen = 1 },<br>
> -   { .tiling = I915_TILING_X, .modifier = I915_FORMAT_MOD_X_TILED,<br>
> -     .since_gen = 1 },<br>
> -   { .tiling = I915_TILING_Y, .modifier = I915_FORMAT_MOD_Y_TILED,<br>
> -     .since_gen = 6 },<br>
> +} supported_modifiers[] = {<br>
> +   { DRM_FORMAT_MOD_LINEAR       , 1 },<br>
> +   { I915_FORMAT_MOD_X_TILED     , 1 },<br>
> +   { I915_FORMAT_MOD_Y_TILED     , 6 },<br>
>  };<br>
<br>
</span>Losing named initialisation makes me very sad. Is there any<br>
buildsys/compiler reason to do this?<br></blockquote><div><br></div><div>Just because it made it one line per modifier.  That said, I think I could probably put them back in.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
The rest LGTM, so with or without ISL:<br>
Reviewed-by: Daniel Stone <<a href="mailto:daniels@collabora.com">daniels@collabora.com</a>><br></blockquote><div><br></div><div>Thanks! <br></div></div><br></div></div>