[Mesa-dev] [PATCH 3/7] i965: Refactor brw_is_hiz_depth_format()
Pohjolainen, Topi
topi.pohjolainen at intel.com
Thu Apr 9 22:17:19 PDT 2015
On Thu, Apr 09, 2015 at 08:57:04PM -0700, Chad Versace wrote:
> Every caller of this function uses it to determine if the current
> miptree needs a hiz buffer to be allocated. Strangely, the function
> doesn't take a miptree argument. So, this function effectively decides
> if and when a miptree's hiz buffer gets allocated without inspecting the
> miptree itself. Luckily, the driver behaves correctly despite the
> brw_is_hiz_depth_format's quirk.
>
> I will soon make some changes to the miptree that will require
> inspecting the miptree to determine if it needs a hiz buffer. So this
> patch renames
> brw_is_hiz_depth_format -> intel_miptree_wants_hiz_buffer
> and gives it a miptree parameter.
>
> This patch shouldn't change any behavior.
>
> Cc: Jordan Justen <jordan.l.justen at intel.com>
> Cc: Tapani Pälli <tapani.palli at intel.com>
> Cc: Zach Reizner <zachr at google.com>
> Cc: Frank Henigman <fjhenigman at google.com>
> Cc: Marta Lofstedt <marta.lofstedt at intel.com>
> Cc: Haixia Shi <hshi at chromium.org>
> ---
> src/mesa/drivers/dri/i965/brw_context.h | 1 -
> src/mesa/drivers/dri/i965/brw_surface_formats.c | 19 ------------------
> src/mesa/drivers/dri/i965/intel_fbo.c | 4 ++--
> src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 26 +++++++++++++++++++++++--
> src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 5 ++++-
> 5 files changed, 30 insertions(+), 25 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h
> index 6c168a3..0bd0ed1 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.h
> +++ b/src/mesa/drivers/dri/i965/brw_context.h
> @@ -1681,7 +1681,6 @@ void brw_upload_abo_surfaces(struct brw_context *brw,
> struct brw_stage_prog_data *prog_data);
>
> /* brw_surface_formats.c */
> -bool brw_is_hiz_depth_format(struct brw_context *ctx, mesa_format format);
> bool brw_render_target_supported(struct brw_context *brw,
> struct gl_renderbuffer *rb);
> uint32_t brw_depth_format(struct brw_context *brw, mesa_format format);
> diff --git a/src/mesa/drivers/dri/i965/brw_surface_formats.c b/src/mesa/drivers/dri/i965/brw_surface_formats.c
> index 7524ad9..c7fb707 100644
> --- a/src/mesa/drivers/dri/i965/brw_surface_formats.c
> +++ b/src/mesa/drivers/dri/i965/brw_surface_formats.c
> @@ -798,22 +798,3 @@ brw_depth_format(struct brw_context *brw, mesa_format format)
> unreachable("Unexpected depth format.");
> }
> }
> -
> -/** Can HiZ be enabled on a depthbuffer of the given format? */
> -bool
> -brw_is_hiz_depth_format(struct brw_context *brw, mesa_format format)
> -{
> - if (!brw->has_hiz)
> - return false;
> -
> - switch (format) {
> - case MESA_FORMAT_Z_FLOAT32:
> - case MESA_FORMAT_Z32_FLOAT_S8X24_UINT:
> - case MESA_FORMAT_Z24_UNORM_X8_UINT:
> - case MESA_FORMAT_Z24_UNORM_S8_UINT:
> - case MESA_FORMAT_Z_UNORM16:
> - return true;
> - default:
> - return false;
> - }
> -}
> diff --git a/src/mesa/drivers/dri/i965/intel_fbo.c b/src/mesa/drivers/dri/i965/intel_fbo.c
> index 2cf4771..7babd29 100644
> --- a/src/mesa/drivers/dri/i965/intel_fbo.c
> +++ b/src/mesa/drivers/dri/i965/intel_fbo.c
> @@ -561,7 +561,7 @@ intel_renderbuffer_update_wrapper(struct brw_context *brw,
>
> intel_renderbuffer_set_draw_offset(irb);
>
> - if (mt->hiz_buf == NULL && brw_is_hiz_depth_format(brw, rb->Format)) {
I had been wondering before if "rb->Format == irb->mt->format" is always true,
and it is. Miptree gets created based on the renderbuffer settings, see
intel_miptree_create_for_renderbuffer().
> + if (intel_miptree_wants_hiz_buffer(brw, mt)) {
> intel_miptree_alloc_hiz(brw, mt);
> if (!mt->hiz_buf)
> return false;
> @@ -1032,7 +1032,7 @@ intel_renderbuffer_move_to_temp(struct brw_context *brw,
> INTEL_MIPTREE_TILING_ANY,
> false);
>
> - if (brw_is_hiz_depth_format(brw, new_mt->format)) {
> + if (intel_miptree_wants_hiz_buffer(brw, new_mt)) {
> intel_miptree_alloc_hiz(brw, new_mt);
> }
>
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> index a906460..492338b 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> @@ -403,7 +403,8 @@ intel_miptree_create_layout(struct brw_context *brw,
> if (!for_bo &&
> _mesa_get_format_base_format(format) == GL_DEPTH_STENCIL &&
> (brw->must_use_separate_stencil ||
> - (brw->has_separate_stencil && brw_is_hiz_depth_format(brw, format)))) {
> + (brw->has_separate_stencil &&
> + intel_miptree_wants_hiz_buffer(brw, mt)))) {
I had to check that format == mt->format, and indeed that is the case. I
noticed that we probably want to make intel_miptree_create_layout() a little
more consistent - it uses randomly both the stack variable and the one in
miptree.
Anyway, patches 1-3 are:
Reviewed-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
> const bool force_all_slices_at_each_lod = brw->gen == 6;
> mt->stencil_mt = intel_miptree_create(brw,
> mt->target,
> @@ -843,7 +844,7 @@ intel_miptree_create_for_renderbuffer(struct brw_context *brw,
> if (!mt)
> goto fail;
>
> - if (brw_is_hiz_depth_format(brw, format)) {
> + if (intel_miptree_wants_hiz_buffer(brw, mt)) {
> ok = intel_miptree_alloc_hiz(brw, mt);
> if (!ok)
> goto fail;
> @@ -1681,6 +1682,27 @@ intel_hiz_miptree_buf_create(struct brw_context *brw,
> return buf;
> }
>
> +bool
> +intel_miptree_wants_hiz_buffer(struct brw_context *brw,
> + struct intel_mipmap_tree *mt)
> +{
> + if (!brw->has_hiz)
> + return false;
> +
> + if (mt->hiz_buf != NULL)
> + return false;
> +
> + switch (mt->format) {
> + case MESA_FORMAT_Z_FLOAT32:
> + case MESA_FORMAT_Z32_FLOAT_S8X24_UINT:
> + case MESA_FORMAT_Z24_UNORM_X8_UINT:
> + case MESA_FORMAT_Z24_UNORM_S8_UINT:
> + case MESA_FORMAT_Z_UNORM16:
> + return true;
> + default:
> + return false;
> + }
> +}
>
> bool
> intel_miptree_alloc_hiz(struct brw_context *brw,
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> index 3c41893..0cb64d2 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> @@ -632,12 +632,15 @@ intel_miptree_copy_teximage(struct brw_context *brw,
> * functions on a miptree without HiZ. In that case, each function is a no-op.
> */
>
> +bool
> +intel_miptree_wants_hiz_buffer(struct brw_context *brw,
> + struct intel_mipmap_tree *mt);
> +
> /**
> * \brief Allocate the miptree's embedded HiZ miptree.
> * \see intel_mipmap_tree:hiz_mt
> * \return false if allocation failed
> */
> -
> bool
> intel_miptree_alloc_hiz(struct brw_context *brw,
> struct intel_mipmap_tree *mt);
> --
> 2.2.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list