On Wed, Jun 10, 2015 at 03:30:50PM -0700, Anuj Phogat wrote:
> In case of I915_TILING_{X,Y} we need to pass tiling format to libdrm
> using drm_intel_bo_alloc_tiled(). But, In case of YF/YS tiled buffers
> libdrm need not know about the tiling format because these buffers
> don't have hardware support to be tiled or detiled through a fenced
> region. libdrm still need to know buffer alignment value for its use
> in kernel when resolving the relocation.
>
> Using drm_intel_bo_alloc_for_render() for YF/YS tiled buffers
> satisfy both the above conditions.
>
> Signed-off-by: Anuj Phogat <anuj.pho...@gmail.com>
> Cc: Ben Widawsky <b...@bwidawsk.net>
> ---
> src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 86
> +++++++++++++++++++++++++--
> 1 file changed, 80 insertions(+), 6 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> index 615cbfb..d4d9e76 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> @@ -522,6 +522,65 @@ intel_lower_compressed_format(struct brw_context *brw,
> mesa_format format)
> }
> }
>
> +/* This function computes Yf/Ys tiled bo size and alignment. */
It also computes pitch for the yf/ys case
> +static uint64_t
> +intel_get_yf_ys_bo_size(struct intel_mipmap_tree *mt, unsigned *alignment)
> +{
> + const uint32_t bpp = mt->cpp * 8;
> + const uint32_t aspect_ratio = (bpp == 16 || bpp == 64) ? 2 : 1;
> + uint32_t tile_width, tile_height;
> + const uint64_t min_size = 512 * 1024;
> + const uint64_t max_size = 64 * 1024 * 1024;
Where do min/max come from? Add a comment?
> + uint64_t i, stride, size, aligned_y;
> +
> + assert(mt->tr_mode != INTEL_MIPTREE_TRMODE_NONE);
> +
> + switch (bpp) {
> + case 8:
> + tile_height = 64;
> + break;
> + case 16:
> + case 32:
> + tile_height = 32;
> + break;
> + case 64:
> + case 128:
> + tile_height = 16;
> + break;
> + default:
> + tile_height = 0;
make this unreachable()
> + printf("Invalid bits per pixel in %s: bpp = %d\n",
> + __FUNCTION__, bpp);
> + }
I think ideally you should roll this logic into intel_miptree_get_tile_masks().
> +
> + if (mt->tr_mode == INTEL_MIPTREE_TRMODE_YS)
> + tile_height *= 4;
> +
> + aligned_y = ALIGN(mt->total_height, tile_height);
> +
> + stride = mt->total_width * mt->cpp;
> + tile_width = tile_height * mt->cpp * aspect_ratio;
> + stride = ALIGN(stride, tile_width);
> + size = stride * aligned_y;
> +
> + if (mt->tr_mode == INTEL_MIPTREE_TRMODE_YF) {
> + *alignment = 4096;
> + size = ALIGN(size, 4096);
> + } else {
> + *alignment = 64 * 1024;
> + size = ALIGN(size, 64 * 1024);
> + }
Hmm. I think the above calculation for size is redundant since you already
aligned to tile_width and height, above. Right? assert((size % 64K) == 0);
> +
> + if (size > max_size) {
> + mt->tr_mode = INTEL_MIPTREE_TRMODE_NONE;
> + return 0;
> + } else {
> + mt->pitch = stride;
> + for (i = min_size; i < size; i <<= 1)
> + ;
> + return i;
I don't understand this. Why don't you just return size? It seems incredibly
wasteful to both start a 512K, and to increment by powers of 2. Did I miss
something?
Also, I don't understand max_size. I must be missing something in the spec with
the min/max values, can you point me to them?
> + }
> +}
>
> struct intel_mipmap_tree *
> intel_miptree_create(struct brw_context *brw,
> @@ -575,12 +634,27 @@ intel_miptree_create(struct brw_context *brw,
>
> unsigned long pitch;
> mt->etc_format = etc_format;
> - mt->bo = drm_intel_bo_alloc_tiled(brw->bufmgr, "miptree",
> - total_width, total_height, mt->cpp,
> - &mt->tiling, &pitch,
> - (expect_accelerated_upload ?
> - BO_ALLOC_FOR_RENDER : 0));
> - mt->pitch = pitch;
> +
> + if (mt->tr_mode != INTEL_MIPTREE_TRMODE_NONE) {
> + unsigned alignment = 0;
> + unsigned long size;
> + size = intel_get_yf_ys_bo_size(mt, &alignment);
> +
> + /* intel_get_yf_ys_bo_size() might change the tr_mode. */
> + if (size > 0 && mt->tr_mode != INTEL_MIPTREE_TRMODE_NONE) {
> + mt->bo = drm_intel_bo_alloc_for_render(brw->bufmgr, "miptree",
> + size, alignment);
> + }
> + }
> +
> + if (mt->tr_mode == INTEL_MIPTREE_TRMODE_NONE) {
> + mt->bo = drm_intel_bo_alloc_tiled(brw->bufmgr, "miptree",
> + total_width, total_height, mt->cpp,
> + &mt->tiling, &pitch,
> + (expect_accelerated_upload ?
> + BO_ALLOC_FOR_RENDER : 0));
> + mt->pitch = pitch;
> + }
>
> /* If the BO is too large to fit in the aperture, we need to use the
> * BLT engine to support it. Prior to Sandybridge, the BLT paths can't
Assuming we can sort out the min/max stuff, and power of 2 size, the rest looks
good.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
