[Mesa-dev] [PATCH 03/23] i965: Separate miptree creation from auxiliary buffer setup
Ben Widawsky
ben at bwidawsk.net
Tue Feb 9 23:05:05 UTC 2016
On Mon, Feb 08, 2016 at 06:51:23PM +0200, Topi Pohjolainen wrote:
> Currently the logic allocating and setting up miptrees is closely
> combined with decision making when to re-allocate buffers in
> X-tiled layout and when to associate colors with auxiliary buffers.
>
> These auxiliary buffers are in turn also represented as miptrees
> and are created by the same miptree creation logic calling itself
> recursively. This means considering in vain if the auxiliary buffers
> should be represented in X-tiled layout or if they should be
> associated with auxiliary buffers again.
> While this is somewhat unnecessary, this doesn't impose any problems
> currently. Miptrees for auxiliary buffers are created as simgle-sampled
> fusing the consideration for multi-sampled compression auxiliary
> buffers. The format in turn is such that is not applicable for
> single-sampled fast clears (that would require accompaning auxiliary
> buffer).
> But once the driver starts to support lossless compression of color
> buffers the auxiliary buffer will have a format that would itself
> be applicable for lossless compression. This would be rather
> difficult and ugly to detect in the current miptree creation logic,
> and therefore this patch seeks to separate the association logic
> from the general allocation and setup steps.
>
> Signed-off-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
Shameless plug. I think we reached the same conclusion about the existing code
:-). I forgot what I actually did, but I'm pretty sure I at least did this
stuff, and maybe some more. I'd love it if you could see if anything is useful
there. https://patchwork.freedesktop.org/patch/56792/
> ---
> src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 75 +++++++++++++++++----------
> 1 file changed, 49 insertions(+), 26 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> index 033f4c6..d655de8 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> @@ -611,17 +611,18 @@ intel_get_yf_ys_bo_size(struct intel_mipmap_tree *mt, unsigned *alignment,
> return size;
> }
>
> -struct intel_mipmap_tree *
> -intel_miptree_create(struct brw_context *brw,
> - GLenum target,
> - mesa_format format,
> - GLuint first_level,
> - GLuint last_level,
> - GLuint width0,
> - GLuint height0,
> - GLuint depth0,
> - GLuint num_samples,
> - uint32_t layout_flags)
> +static struct intel_mipmap_tree *
> +miptree_create(struct brw_context *brw,
> + GLenum target,
> + mesa_format format,
> + GLuint first_level,
> + GLuint last_level,
> + GLuint width0,
> + GLuint height0,
> + GLuint depth0,
> + GLuint num_samples,
> + enum intel_msaa_layout msaa_layout,
> + uint32_t layout_flags)
> {
> struct intel_mipmap_tree *mt;
> mesa_format tex_format = format;
> @@ -638,9 +639,7 @@ intel_miptree_create(struct brw_context *brw,
> mt = intel_miptree_create_layout(brw, target, format,
> first_level, last_level, width0,
> height0, depth0, num_samples,
> - compute_msaa_layout(brw, format,
> - num_samples, false),
> - layout_flags);
> + msaa_layout, layout_flags);
> /*
> * pitch == 0 || height == 0 indicates the null texture
> */
> @@ -658,12 +657,8 @@ intel_miptree_create(struct brw_context *brw,
> total_height = ALIGN(total_height, 64);
> }
>
> - bool y_or_x = false;
> -
> - if (mt->tiling == (I915_TILING_Y | I915_TILING_X)) {
> - y_or_x = true;
> + if (mt->tiling == (I915_TILING_Y | I915_TILING_X))
> mt->tiling = I915_TILING_Y;
> - }
>
> if (layout_flags & MIPTREE_LAYOUT_ACCELERATED_UPLOAD)
> alloc_flags |= BO_ALLOC_FOR_RENDER;
> @@ -686,12 +681,46 @@ intel_miptree_create(struct brw_context *brw,
> }
>
> mt->pitch = pitch;
> + mt->offset = 0;
> +
> + if (!mt->bo) {
> + intel_miptree_release(&mt);
> + return NULL;
> + }
> +
> + return mt;
> +}
> +
> +struct intel_mipmap_tree *
> +intel_miptree_create(struct brw_context *brw,
> + GLenum target,
> + mesa_format format,
> + GLuint first_level,
> + GLuint last_level,
> + GLuint width0,
> + GLuint height0,
> + GLuint depth0,
> + GLuint num_samples,
> + uint32_t layout_flags)
> +{
> + struct intel_mipmap_tree *mt = miptree_create(
> + brw, target, format,
> + first_level, last_level,
> + width0, height0, depth0, num_samples,
> + compute_msaa_layout(brw, format,
> + num_samples, false),
> + layout_flags);
>
> /* 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
> * handle Y-tiling, so we need to fall back to X.
> */
> + const bool y_or_x = mt->tiling == (I915_TILING_Y | I915_TILING_X);
> if (brw->gen < 6 && y_or_x && mt->bo->size >= brw->max_gtt_map_object_size) {
> + unsigned long pitch = mt->pitch;
> + const uint32_t alloc_flags =
> + (layout_flags & MIPTREE_LAYOUT_ACCELERATED_UPLOAD) ?
> + BO_ALLOC_FOR_RENDER : 0;
mt may be NULL if miptree_create fails. SO I think you need an
if (!mt)
return NULL;
> perf_debug("%dx%d miptree larger than aperture; falling back to X-tiled\n",
> mt->total_width, mt->total_height);
>
> @@ -700,14 +729,8 @@ intel_miptree_create(struct brw_context *brw,
> mt->bo = drm_intel_bo_alloc_tiled(brw->bufmgr, "miptree",
> mt->total_width, mt->total_height, mt->cpp,
> &mt->tiling, &pitch, alloc_flags);
> - mt->pitch = pitch;
> - }
> -
> - mt->offset = 0;
>
> - if (!mt->bo) {
> - intel_miptree_release(&mt);
> - return NULL;
> + mt->pitch = pitch;
You could get rid of pitch local variable and just pass &mt->pitch, if you want.
> }
>
>
More information about the mesa-dev
mailing list