<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Sep 10, 2015 at 12:20 PM, Chad Versace <span dir="ltr"><<a href="mailto:chad.versace@intel.com" target="_blank">chad.versace@intel.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Wed 19 Aug 2015, Anuj Phogat wrote:<br>
> V2:<br>
> - Do the tile width/height computations in the new helper<br>
> function and use it later in intel_miptree_get_tile_masks().<br>
> - Change the name to intel_get_tile_dims().<br>
><br>
> Cc: Ben Widawsky <<a href="mailto:ben@bwidawsk.net">ben@bwidawsk.net</a>><br>
> Signed-off-by: Anuj Phogat <<a href="mailto:anuj.phogat@gmail.com">anuj.phogat@gmail.com</a>><br>
> ---<br>
> src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 81 +++++++++++++++++++--------<br>
> src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 4 ++<br>
> 2 files changed, 63 insertions(+), 22 deletions(-)<br>
><br>
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c<br>
> index e85c3f0..c282e94 100644<br>
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c<br>
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c<br>
> @@ -563,35 +563,15 @@ static unsigned long<br>
> intel_get_yf_ys_bo_size(struct intel_mipmap_tree *mt, unsigned *alignment,<br>
> unsigned long *pitch)<br>
> {<br>
> - const uint32_t bpp = mt->cpp * 8;<br>
> - const uint32_t aspect_ratio = (bpp == 16 || bpp == 64) ? 2 : 1;<br>
> uint32_t tile_width, tile_height;<br>
> unsigned long stride, size, aligned_y;<br>
><br>
> assert(mt->tr_mode != INTEL_MIPTREE_TRMODE_NONE);<br>
> -<br>
> - switch (bpp) {<br>
> - case 8:<br>
> - tile_height = 64;<br>
> - break;<br>
> - case 16:<br>
> - case 32:<br>
> - tile_height = 32;<br>
> - break;<br>
> - case 64:<br>
> - case 128:<br>
> - tile_height = 16;<br>
> - break;<br>
> - default:<br>
> - unreachable("not reached");<br>
> - }<br>
> -<br>
> - if (mt->tr_mode == INTEL_MIPTREE_TRMODE_YS)<br>
> - tile_height *= 4;<br>
> + intel_get_tile_dims(mt->tiling, mt->tr_mode, mt->cpp,<br>
> + &tile_width, &tile_height);<br>
><br>
> aligned_y = ALIGN(mt->total_height, tile_height);<br>
> stride = mt->total_width * mt->cpp;<br>
> - tile_width = tile_height * mt->cpp * aspect_ratio;<br>
> stride = ALIGN(stride, tile_width);<br>
> size = stride * aligned_y;<br>
><br>
> @@ -1081,6 +1061,63 @@ intel_miptree_get_image_offset(const struct intel_mipmap_tree *mt,<br>
> *y = mt->level[level].slice[slice].y_offset;<br>
> }<br>
><br>
> +<br>
> +/**<br>
> + * This function computes the width and height in bytes of different tiling<br>
> + * patterns. If the BO is untiled, the dimensions are set to cpp.<br>
> + */<br>
<br>
</div></div>Is the tile_w parameter in units of bytes or pixels? That should be<br>
documented at the top of the function.<br></blockquote><div>It's in bytes. I'll document it.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Also, just to be clear, "tile height" is always unitless. The hw docs<br>
sometime express it in units of "rows". But "rows" itself is unitless.<br>
<span class=""><br></span></blockquote><div>Right. I'm returning the tile_h in bytes in this function, so I need to fix it.</div><div>It didn't break anything because It isn't used anywhere.</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
> +void<br>
> +intel_get_tile_dims(uint32_t tiling, uint32_t tr_mode, uint32_t cpp,<br>
> + uint32_t *tile_w, uint32_t *tile_h)<br>
> +{<br>
> + if (tr_mode == INTEL_MIPTREE_TRMODE_NONE) {<br>
> + switch (tiling) {<br>
> + case I915_TILING_X:<br>
> + *tile_w = 512;<br>
> + *tile_h = 8 * cpp;<br>
<br>
</span>For legacy tiling formats, the height of a tile is independent of the<br>
pixel size, because the height is unitless. For Tile X, it's always<br>
2^3. For Tile Y Legacy, it's always 2^5.<br></blockquote><div>Right. I'll fix it. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
If tile_w is in units of bytes, then it's also independent of pixel<br>
size. If tile_w is in units of pixels, though, then<br>
<br>
tile_w_pixels = tile_w_bytes / cpp<br>
<span class=""><br>
<br>
> + break;<br>
> + case I915_TILING_Y:<br>
> + *tile_w = 128;<br>
> + *tile_h = 32 * cpp;<br>
> + break;<br>
> + case I915_TILING_NONE:<br>
> + *tile_w = cpp;<br>
> + *tile_h = cpp;<br>
> + break;<br>
> + default:<br>
> + unreachable("not reached");<br>
> + }<br>
> + } else {<br>
> + uint32_t aspect_ratio = 1;<br>
> + assert(_mesa_is_pow_two(cpp));<br>
> +<br>
> + switch (cpp) {<br>
> + case 1:<br>
> + *tile_h = 64 * cpp;<br>
<br>
</span>I'm still reading the docs for the non-legay tiling formats Yf, and Ys.<br>
So I can't comment on this part of the patch.<br>
<div class="HOEnZb"><div class="h5"><br>
> + break;<br>
> + case 2:<br>
> + case 4:<br>
> + *tile_h = 32 * cpp;<br>
> + break;<br>
> + case 8:<br>
> + case 16:<br>
> + *tile_h = 16 * cpp;<br>
> + break;<br>
> + default:<br>
> + unreachable("not reached");<br>
> + }<br>
> +<br>
> + if (cpp == 2 || cpp == 8)<br>
> + aspect_ratio = 2;<br>
> +<br>
> + if (tr_mode == INTEL_MIPTREE_TRMODE_YS)<br>
> + *tile_h *= 4;<br>
> +<br>
> + *tile_w = *tile_h * aspect_ratio;<br>
> + }<br>
> +}<br>
</div></div></blockquote></div><br></div></div>