[Mesa-dev] [PATCH] i965/Gen7: Fix HiZ ops for MSAA depth
Previously, we would program the sample count based on the blorp
operation's dst sample count -- which is zero for a HiZ op; we'd also
ignore the difference between physical and logical surface size for
the depth surface.
That kindof worked -- the two errors almost cancel out -- but produced
strange blocky artifacts.
Instead, program the sample count properly, and use the logical
dimensions of the depth surface.
Fixes broken rendering in `Tesseract` with msaa=4 glineardepth=0
NOTE: I've hacked the BLORP depth alignment code a bit (at `Not quite
sure ...`). This is almost certainly not the right thing.
Signed-off-by: Chris Forbes
---
src/mesa/drivers/dri/i965/gen7_blorp.cpp | 41 +++-
1 file changed, 25 insertions(+), 16 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/gen7_blorp.cpp
b/src/mesa/drivers/dri/i965/gen7_blorp.cpp
index 4bf9396..58dc497 100644
--- a/src/mesa/drivers/dri/i965/gen7_blorp.cpp
+++ b/src/mesa/drivers/dri/i965/gen7_blorp.cpp
@@ -447,7 +447,8 @@ gen7_blorp_emit_streamout_disable(struct brw_context *brw,
static void
gen7_blorp_emit_sf_config(struct brw_context *brw,
- const brw_blorp_params *params)
+ const brw_blorp_params *params,
+ int num_samples)
{
/* 3DSTATE_SF
*
@@ -472,7 +473,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(num_samples > 1 ? GEN6_SF_MSRAST_ON_PATTERN : 0);
OUT_BATCH(0);
OUT_BATCH(0);
OUT_BATCH(0);
@@ -500,7 +501,8 @@ gen7_blorp_emit_sf_config(struct brw_context *brw,
static void
gen7_blorp_emit_wm_config(struct brw_context *brw,
const brw_blorp_params *params,
- brw_blorp_prog_data *prog_data)
+ brw_blorp_prog_data *prog_data,
+ int num_samples)
{
uint32_t dw1 = 0, dw2 = 0;
@@ -528,7 +530,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 (num_samples > 1) {
dw1 |= GEN7_WM_MSRAST_ON_PATTERN;
if (prog_data && prog_data->persample_msaa_dispatch)
dw2 |= GEN7_WM_MSDISPMODE_PERSAMPLE;
@@ -562,7 +564,8 @@ static void
gen7_blorp_emit_ps_config(struct brw_context *brw,
const brw_blorp_params *params,
uint32_t prog_offset,
- brw_blorp_prog_data *prog_data)
+ brw_blorp_prog_data *prog_data,
+ int num_samples)
{
uint32_t dw2, dw4, dw5;
const int max_threads_shift = brw->is_haswell ?
@@ -579,8 +582,10 @@ gen7_blorp_emit_ps_config(struct brw_context *brw,
*/
dw4 |= GEN7_PS_16_DISPATCH_ENABLE;
- if (brw->is_haswell)
- dw4 |= SET_FIELD(1, HSW_PS_SAMPLE_MASK); /* 1 sample for now */
+ if (brw->is_haswell) {
+ int sample_mask = (num_samples > 1) ? (1 << num_samples) - 1 : 1;
+ dw4 |= SET_FIELD(sample_mask, HSW_PS_SAMPLE_MASK);
+ }
if (params->use_wm_prog) {
dw2 |= 1 << GEN7_PS_SAMPLER_COUNT_SHIFT; /* Up to 4 samplers */
dw4 |= GEN7_PS_PUSH_CONSTANT_ENABLE;
@@ -714,7 +719,9 @@ gen7_blorp_emit_depth_stencil_config(struct brw_context
*brw,
lod = params->depth.level - params->depth.mt->first_level;
- if (params->hiz_op != GEN6_HIZ_OP_NONE && lod == 0) {
+ /* not quite sure about the MSAA interaction here! */
+ if (params->hiz_op != GEN6_HIZ_OP_NONE && lod == 0 &&
+ params->depth.mt->num_samples <= 1) {
/* HIZ ops for lod 0 may set the width & height a little
* larger to allow the fast depth clear to fit the hardware
* alignment requirements. (8x4)
@@ -722,8 +729,8 @@ gen7_blorp_emit_depth_stencil_config(struct brw_context
*brw,
surfwidth = params->depth.width;
surfheight = params->depth.height;
} else {
- surfwidth = params->depth.mt->logical_width0;
- surfheight = params->depth.mt->logical_height0;
+ surfwidth = ALIGN(params->depth.mt->logical_width0, 8);
+ surfheight = ALIGN(params->depth.mt->logical_height0, 4);
}
/* 3DSTATE_DEPTH_BUFFER */
@@ -863,10 +870,12 @@ 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);
+ int num_samples = (params->hiz_op == GEN6_HIZ_OP_NONE) ?
+ params->dst.num_samples : params->depth.mt->num_samples;
+ gen6_emit_3dstate_multisample(brw, num_samples);
gen6_emit_3dstate_sample_mask(brw,
- params->dst.num_samples > 1 ?
-
Re: [Mesa-dev] [PATCH] i965/Gen7: Fix HiZ ops for MSAA depth
Ping.
This has been broken for ages, and at least tesseract is carrying
hacks to work around it (extra color buffer containing depth values,
in their g-buffer). Getting rid of that wins us about 15%.
-- Chris
On Sat, Feb 8, 2014 at 3:54 PM, Chris Forbes wrote:
> Previously, we would program the sample count based on the blorp
> operation's dst sample count -- which is zero for a HiZ op; we'd also
> ignore the difference between physical and logical surface size for
> the depth surface.
>
> That kindof worked -- the two errors almost cancel out -- but produced
> strange blocky artifacts.
>
> Instead, program the sample count properly, and use the logical
> dimensions of the depth surface.
>
> Fixes broken rendering in `Tesseract` with msaa=4 glineardepth=0
>
> NOTE: I've hacked the BLORP depth alignment code a bit (at `Not quite
> sure ...`). This is almost certainly not the right thing.
>
> Signed-off-by: Chris Forbes
> ---
> src/mesa/drivers/dri/i965/gen7_blorp.cpp | 41
> +++-
> 1 file changed, 25 insertions(+), 16 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> b/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> index 4bf9396..58dc497 100644
> --- a/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> +++ b/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> @@ -447,7 +447,8 @@ gen7_blorp_emit_streamout_disable(struct brw_context *brw,
>
> static void
> gen7_blorp_emit_sf_config(struct brw_context *brw,
> - const brw_blorp_params *params)
> + const brw_blorp_params *params,
> + int num_samples)
> {
> /* 3DSTATE_SF
> *
> @@ -472,7 +473,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(num_samples > 1 ? GEN6_SF_MSRAST_ON_PATTERN : 0);
>OUT_BATCH(0);
>OUT_BATCH(0);
>OUT_BATCH(0);
> @@ -500,7 +501,8 @@ gen7_blorp_emit_sf_config(struct brw_context *brw,
> static void
> gen7_blorp_emit_wm_config(struct brw_context *brw,
>const brw_blorp_params *params,
> - brw_blorp_prog_data *prog_data)
> + brw_blorp_prog_data *prog_data,
> + int num_samples)
> {
> uint32_t dw1 = 0, dw2 = 0;
>
> @@ -528,7 +530,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 (num_samples > 1) {
> dw1 |= GEN7_WM_MSRAST_ON_PATTERN;
> if (prog_data && prog_data->persample_msaa_dispatch)
> dw2 |= GEN7_WM_MSDISPMODE_PERSAMPLE;
> @@ -562,7 +564,8 @@ static void
> gen7_blorp_emit_ps_config(struct brw_context *brw,
>const brw_blorp_params *params,
>uint32_t prog_offset,
> - brw_blorp_prog_data *prog_data)
> + brw_blorp_prog_data *prog_data,
> + int num_samples)
> {
> uint32_t dw2, dw4, dw5;
> const int max_threads_shift = brw->is_haswell ?
> @@ -579,8 +582,10 @@ gen7_blorp_emit_ps_config(struct brw_context *brw,
> */
> dw4 |= GEN7_PS_16_DISPATCH_ENABLE;
>
> - if (brw->is_haswell)
> - dw4 |= SET_FIELD(1, HSW_PS_SAMPLE_MASK); /* 1 sample for now */
> + if (brw->is_haswell) {
> + int sample_mask = (num_samples > 1) ? (1 << num_samples) - 1 : 1;
> + dw4 |= SET_FIELD(sample_mask, HSW_PS_SAMPLE_MASK);
> + }
> if (params->use_wm_prog) {
>dw2 |= 1 << GEN7_PS_SAMPLER_COUNT_SHIFT; /* Up to 4 samplers */
>dw4 |= GEN7_PS_PUSH_CONSTANT_ENABLE;
> @@ -714,7 +719,9 @@ gen7_blorp_emit_depth_stencil_config(struct brw_context
> *brw,
>
> lod = params->depth.level - params->depth.mt->first_level;
>
> - if (params->hiz_op != GEN6_HIZ_OP_NONE && lod == 0) {
> + /* not quite sure about the MSAA interaction here! */
> + if (params->hiz_op != GEN6_HIZ_OP_NONE && lod == 0 &&
> + params->depth.mt->num_samples <= 1) {
>/* HIZ ops for lod 0 may set the width & height a little
> * larger to allow the fast depth clear to fit the hardware
> * alignment requirements. (8x4)
> @@ -722,8 +729,8 @@ gen7_blorp_emit_depth_stencil_config(struct brw_context
> *brw,
>surfwidth = params->depth.width;
>surfheight = params->depth.height;
> } else {
> - surfwidth = params->depth.mt->logical_width0;
> - surfheight = params->depth.mt->logical_height0;
> + surfwidth = ALIGN(params->depth.mt->logical_width0, 8);
> + surfheight = ALIGN(params->depth.mt->logical_height0, 4);
> }
>
> /* 3DSTATE_DEPTH_BUFFER */
> @@ -863,10 +870,12 @@ gen7_blorp_exec(
