[Mesa-dev] [PATCH] i965/blorp: Fix hiz ops on MSAA surfaces

[Anuj Phogat]

On Tue, Nov 4, 2014 at 8:40 AM, Chris Forbes <chrisf at ijw.co.nz> wrote:

> Two things were broken here:
> - The depth/stencil surface dimensions were broken for MSAA.
> - Sample count was programmed incorrectly.
>
> Result was the depth resolve didn't work correctly on MSAA surfaces, and
> so sampling the surface later produced garbage.
>
> Fixes the new piglit test arb_texture_multisample-sample-depth, and
> various artifacts in 'tesseract' with msaa=4 glineardepth=0.
>
> Not observed any piglit regressions on Haswell.
>
> Signed-off-by: Chris Forbes <chrisf at ijw.co.nz>
> ---
>  src/mesa/drivers/dri/i965/brw_blorp.h    |  4 ++++
>  src/mesa/drivers/dri/i965/gen7_blorp.cpp | 24 +++++++++++++++++-------
>  2 files changed, 21 insertions(+), 7 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_blorp.h
> b/src/mesa/drivers/dri/i965/brw_blorp.h
> index ff68000..c4ff0f7 100644
> --- a/src/mesa/drivers/dri/i965/brw_blorp.h
> +++ b/src/mesa/drivers/dri/i965/brw_blorp.h
> @@ -236,6 +236,10 @@ public:
>     bool use_wm_prog;
>     brw_blorp_wm_push_constants wm_push_consts;
>     bool color_write_disable[4];
> +
> +   unsigned num_samples() const {
> +      return dst.mt ? dst.num_samples : depth.mt->num_samples;
> +   }
>  };
>
>
> diff --git a/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> b/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> index 206a6ff..cc57ffe 100644
> --- a/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> +++ b/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> @@ -415,7 +415,7 @@ gen7_blorp_emit_sf_config(struct brw_context *brw,
>        OUT_BATCH(_3DSTATE_SF << 16 | (7 - 2));
>        OUT_BATCH(params->depth_format <<
>                  GEN7_SF_DEPTH_BUFFER_SURFACE_FORMAT_SHIFT);
> -      OUT_BATCH(params->dst.num_samples > 1 ? GEN6_SF_MSRAST_ON_PATTERN :
> 0);
> +      OUT_BATCH(params->num_samples() > 1 ? GEN6_SF_MSRAST_ON_PATTERN :
> 0);
>        OUT_BATCH(0);
>        OUT_BATCH(0);
>        OUT_BATCH(0);
> @@ -470,7 +470,7 @@ gen7_blorp_emit_wm_config(struct brw_context *brw,
>        dw1 |= GEN7_WM_DISPATCH_ENABLE; /* We are rendering */
>     }
>
> -      if (params->dst.num_samples > 1) {
> +      if (params->num_samples() > 1) {
>           dw1 |= GEN7_WM_MSRAST_ON_PATTERN;
>           if (prog_data && prog_data->persample_msaa_dispatch)
>              dw2 |= GEN7_WM_MSDISPMODE_PERSAMPLE;
> @@ -661,8 +661,17 @@ gen7_blorp_emit_depth_stencil_config(struct
> brw_context *brw,
>         * larger to allow the fast depth clear to fit the hardware
>         * alignment requirements. (8x4)
>         */
> -      surfwidth = params->depth.width;
> -      surfheight = params->depth.height;
> +
> +      if (params->num_samples() > 1) {
> +         /* If this is an MSAA + HIZ op, we need to program the
> +          * aligned logical size of the depth surface.
> +          */
> +         surfwidth = ALIGN(params->depth.mt->logical_width0, 8);
> +         surfheight = ALIGN(params->depth.mt->logical_height0, 4);
> +      } else {
> +         surfwidth = params->depth.width;
> +         surfheight = params->depth.height;
> +      }
>     } else {
>        surfwidth = params->depth.mt->logical_width0;
>        surfheight = params->depth.mt->logical_height0;
> @@ -805,10 +814,11 @@ gen7_blorp_exec(struct brw_context *brw,
>     uint32_t sampler_offset = 0;
>
>     uint32_t prog_offset = params->get_wm_prog(brw, &prog_data);
> -   gen6_emit_3dstate_multisample(brw, params->dst.num_samples);
> +   unsigned num_samples = params->num_samples();
> +   gen6_emit_3dstate_multisample(brw, num_samples);
>     gen6_emit_3dstate_sample_mask(brw,
> -                                 params->dst.num_samples > 1 ?
> -                                 (1 << params->dst.num_samples) - 1 : 1);
> +                                 num_samples > 1 ?
> +                                 (1 << num_samples) - 1 : 1);
>     gen6_blorp_emit_state_base_address(brw, params);
>     gen6_blorp_emit_vertices(brw, params);
>     gen7_blorp_emit_urb_config(brw, params);
> --
> 2.1.2
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>

Looks good to me. Verified the requirement in IVB PRM.
Reviewed-by: Anuj Phogat <anuj.phogat at gmail.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20141106/1c9905a1/attachment-0001.html>