On Tuesday, June 23, 2015 01:23:05 PM 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.
>
> V2: Delete min/max buffer size restrictions not valid for i965+.
> Remove redundant align to tile size statements.
> Remove some redundant code now when there are no min/max buffer size.
>
> Signed-off-by: Anuj Phogat <anuj.pho...@gmail.com>
> Cc: Ben Widawsky <b...@bwidawsk.net>
> ---
> src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 62
> +++++++++++++++++++++++++--
> 1 file changed, 58 insertions(+), 4 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> index 80c52f2..5bcb094 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> @@ -558,6 +558,48 @@ intel_lower_compressed_format(struct brw_context *brw,
> mesa_format format)
> }
> }
>
> +/* This function computes Yf/Ys tiled bo size, alignment and pitch. */
> +static uint64_t
> +intel_get_yf_ys_bo_size(struct intel_mipmap_tree *mt, unsigned *alignment,
> + uint64_t *pitch)
Hi Anuj,
This patch has a subtle bug: you've specified pitch and stride to be
uint64_t here, but below when you call it....
[snip]
> @@ -616,11 +658,23 @@ intel_miptree_create(struct brw_context *brw,
> alloc_flags |= BO_ALLOC_FOR_RENDER;
>
> unsigned long pitch;
> - mt->bo = drm_intel_bo_alloc_tiled(brw->bufmgr, "miptree", total_width,
> - total_height, mt->cpp, &mt->tiling,
> - &pitch, alloc_flags);
> mt->etc_format = etc_format;
> - 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, &pitch);
...you're passing a pointer to an unsigned long. On 32-bit builds,
unsigned long is a 4 byte value, while uint64_t is 8 bytes. This could
lead to stack corruption. (GCC warns about this during a 32-bit build.)
I assumed the solution was to make everything uint32_t, but apparently
drm_intel_bo_alloc_tiled actually expects an unsigned long. So we can't
change that.
Then I looked at your code, and realized that nothing even uses the
pitch value. Is there some point to the parameter existing at all?
--Ken
> + assert(size);
> + mt->bo = drm_intel_bo_alloc_for_render(brw->bufmgr, "miptree",
> + size, alignment);
> + mt->pitch = pitch;
> + } else {
> + mt->bo = drm_intel_bo_alloc_tiled(brw->bufmgr, "miptree",
> + total_width, total_height, mt->cpp,
> + &mt->tiling, &pitch,
> + alloc_flags);
> + 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
>
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
