[Mesa-dev] [PATCH 1/6] i965: Kill y_or_x variable in miptree tiling selection

[Neil Roberts]

Ben Widawsky <benjamin.widawsky at intel.com> writes:

> This patch actually addresses two potential issues:
> 1.  As far as I can tell, drm_intel_gem_bo_alloc_tiled() which is invariably
> called, can potentially change the tiling type. Therefore, we shouldn't be
> checking the requested tiling type, but rather the granted tiling
> 2. The existing code can fall back to X-tiled even if choose_tiling said
> X-tiling was not okay.

I'm not sure about that last point. Previously it was like this:

  if (... && y_or_x && ...)
     /* fall back to X */

y_or_x is only set if choose_tiling explicity reported that both X and Y
tiling are ok.

I think with the changes it actually makes it more likely to disobey
choose_tiling. For example, if you pass INTEL_MIPTREE_TILING_ANY for the
requested tiling of a depth texture then choose_tiling will force it to
be only y-tiling. With this patch that aperture check can now try to
override that to x-tiling which presumably would break things.

It seems like this area needs a lot more work than just this patch. I
wonder if this patch isn't necessary for the rest of the series then it
might be better to leave it until we can do a more complete cleanup.

I think choose_tiling shouldn't be passed the requested tiling becase
that seems to just completely override any descisions choose_tiling
makes so it's a bit pointless even asking the function what's possible.
Maybe it should just call choose_tiling and then verify that the
requested tiling is one of the ones that choose_tiling allows. It could
to the same to verify that drm_intel_gem_bo_alloc_tiled gives us one of
the allowed layouts. I guess that would imply having some more error
conditions for combinations that simply can't be done.

- Neil