[Mesa-dev] [PATCH] i965/blorp: Fix hiz ops on MSAA surfaces
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
---
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@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] i965/blorp: Fix hiz ops on MSAA surfaces
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.
v2: Just set brw_hiz_op_params::dst.num_samples rather than adding a
helper function (Ken).
Signed-off-by: Chris Forbes
---
src/mesa/drivers/dri/i965/brw_blorp.cpp | 1 +
src/mesa/drivers/dri/i965/gen7_blorp.cpp | 11 +++
2 files changed, 12 insertions(+)
diff --git a/src/mesa/drivers/dri/i965/brw_blorp.cpp
b/src/mesa/drivers/dri/i965/brw_blorp.cpp
index 20ce7b7..0ecf330 100644
--- a/src/mesa/drivers/dri/i965/brw_blorp.cpp
+++ b/src/mesa/drivers/dri/i965/brw_blorp.cpp
@@ -323,6 +323,7 @@ brw_hiz_op_params::brw_hiz_op_params(struct
intel_mipmap_tree *mt,
*/
depth.width = ALIGN(depth.width, 8);
depth.height = ALIGN(depth.height, 4);
+ dst.num_samples = mt->num_samples;
x1 = depth.width;
y1 = depth.height;
diff --git a/src/mesa/drivers/dri/i965/gen7_blorp.cpp
b/src/mesa/drivers/dri/i965/gen7_blorp.cpp
index 206a6ff..03fc9c8 100644
--- a/src/mesa/drivers/dri/i965/gen7_blorp.cpp
+++ b/src/mesa/drivers/dri/i965/gen7_blorp.cpp
@@ -663,6 +663,17 @@ gen7_blorp_emit_depth_stencil_config(struct brw_context
*brw,
*/
surfwidth = params->depth.width;
surfheight = params->depth.height;
+
+ if (params->dst.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;
--
2.1.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965/blorp: Fix hiz ops on MSAA surfaces
On Tue, Nov 4, 2014 at 8:40 AM, Chris Forbes 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
> ---
> 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@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
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965/blorp: Fix hiz ops on MSAA surfaces
Thanks for the review. Ken has a slightly cleaner version of the patch
which avoids adding the helper function.
On Nov 7, 2014 2:29 AM, "Anuj Phogat" wrote:
>
>
> On Tue, Nov 4, 2014 at 8:40 AM, Chris Forbes 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
>> ---
>> 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@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
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965/blorp: Fix hiz ops on MSAA surfaces
On Tuesday, November 18, 2014 09:49:53 PM Chris Forbes 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.
>
> v2: Just set brw_hiz_op_params::dst.num_samples rather than adding a
> helper function (Ken).
>
> Signed-off-by: Chris Forbes
> ---
> src/mesa/drivers/dri/i965/brw_blorp.cpp | 1 +
> src/mesa/drivers/dri/i965/gen7_blorp.cpp | 11 +++
> 2 files changed, 12 insertions(+)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_blorp.cpp
b/src/mesa/drivers/dri/i965/brw_blorp.cpp
> index 20ce7b7..0ecf330 100644
> --- a/src/mesa/drivers/dri/i965/brw_blorp.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_blorp.cpp
> @@ -323,6 +323,7 @@ brw_hiz_op_params::brw_hiz_op_params(struct
intel_mipmap_tree *mt,
> */
> depth.width = ALIGN(depth.width, 8);
> depth.height = ALIGN(depth.height, 4);
> + dst.num_samples = mt->num_samples;
>
> x1 = depth.width;
> y1 = depth.height;
> diff --git a/src/mesa/drivers/dri/i965/gen7_blorp.cpp
b/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> index 206a6ff..03fc9c8 100644
> --- a/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> +++ b/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> @@ -663,6 +663,17 @@ gen7_blorp_emit_depth_stencil_config(struct brw_context
*brw,
> */
>surfwidth = params->depth.width;
>surfheight = params->depth.height;
> +
> + if (params->dst.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;
>
Jordan, Chad...could one of you take a look at this? It looks reasonable, but
I always get confused by when/where we do rounding in BLORP.
Chris - we might want to Cc this to stable too.
--Ken
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
Re: [Mesa-dev] [PATCH] i965/blorp: Fix hiz ops on MSAA surfaces
On 2014-11-18 00:56:02, Kenneth Graunke wrote:
> On Tuesday, November 18, 2014 09:49:53 PM Chris Forbes 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.
> >
> > v2: Just set brw_hiz_op_params::dst.num_samples rather than adding a
> > helper function (Ken).
> >
> > Signed-off-by: Chris Forbes
> > ---
> > src/mesa/drivers/dri/i965/brw_blorp.cpp | 1 +
> > src/mesa/drivers/dri/i965/gen7_blorp.cpp | 11 +++
> > 2 files changed, 12 insertions(+)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_blorp.cpp
> b/src/mesa/drivers/dri/i965/brw_blorp.cpp
> > index 20ce7b7..0ecf330 100644
> > --- a/src/mesa/drivers/dri/i965/brw_blorp.cpp
> > +++ b/src/mesa/drivers/dri/i965/brw_blorp.cpp
> > @@ -323,6 +323,7 @@ brw_hiz_op_params::brw_hiz_op_params(struct
> intel_mipmap_tree *mt,
> > */
> > depth.width = ALIGN(depth.width, 8);
> > depth.height = ALIGN(depth.height, 4);
> > + dst.num_samples = mt->num_samples;
> >
> > x1 = depth.width;
> > y1 = depth.height;
> > diff --git a/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> b/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> > index 206a6ff..03fc9c8 100644
> > --- a/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> > +++ b/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> > @@ -663,6 +663,17 @@ gen7_blorp_emit_depth_stencil_config(struct
> > brw_context
> *brw,
> > */
> >surfwidth = params->depth.width;
> >surfheight = params->depth.height;
> > +
> > + if (params->dst.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);
Would something like this cover all cases?
surfwidth = params->depth.mt->logical_width0;
surfheight = params->depth.mt->logical_height0;
if (params->hiz_op != GEN6_HIZ_OP_NONE && lod == 0) {
surfwidth = ALIGN(surfwidth, 8);
surfheight = ALIGN(surfheight, 4);
}
> > + } else {
> > + surfwidth = params->depth.width;
> > + surfheight = params->depth.height;
Duplicate of the code just above.
-Jordan
> > + }
> > } else {
> >surfwidth = params->depth.mt->logical_width0;
> >surfheight = params->depth.mt->logical_height0;
> >
>
> Jordan, Chad...could one of you take a look at this? It looks reasonable,
> but
> I always get confused by when/where we do rounding in BLORP.
>
> Chris - we might want to Cc this to stable too.
>
> --Ken
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965/blorp: Fix hiz ops on MSAA surfaces
On Tue 18 Nov 2014, Chris Forbes 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.
v2: Just set brw_hiz_op_params::dst.num_samples rather than adding a
helper function (Ken).
Signed-off-by: Chris Forbes
---
src/mesa/drivers/dri/i965/brw_blorp.cpp | 1 +
src/mesa/drivers/dri/i965/gen7_blorp.cpp | 11 +++
2 files changed, 12 insertions(+)
diff --git a/src/mesa/drivers/dri/i965/brw_blorp.cpp
b/src/mesa/drivers/dri/i965/brw_blorp.cpp
index 20ce7b7..0ecf330 100644
--- a/src/mesa/drivers/dri/i965/brw_blorp.cpp
+++ b/src/mesa/drivers/dri/i965/brw_blorp.cpp
@@ -323,6 +323,7 @@ brw_hiz_op_params::brw_hiz_op_params(struct
intel_mipmap_tree *mt,
*/
depth.width = ALIGN(depth.width, 8);
depth.height = ALIGN(depth.height, 4);
+ dst.num_samples = mt->num_samples;
x1 = depth.width;
y1 = depth.height;
diff --git a/src/mesa/drivers/dri/i965/gen7_blorp.cpp
b/src/mesa/drivers/dri/i965/gen7_blorp.cpp
index 206a6ff..03fc9c8 100644
--- a/src/mesa/drivers/dri/i965/gen7_blorp.cpp
+++ b/src/mesa/drivers/dri/i965/gen7_blorp.cpp
@@ -663,6 +663,17 @@ gen7_blorp_emit_depth_stencil_config(struct brw_context
*brw,
*/
surfwidth = params->depth.width;
surfheight = params->depth.height;
+
+ if (params->dst.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;
+ }
I think the code that aligns up to 8x4 should go into hunk above that
touches brw_hiz_op_params::brw_hiz_op_params(), because:
1. That function already does the 8x4 alignment for the non-MSAA
case. It doesn't make sense to do the 8x4 alignment for MSAA as
a special case in a different file.
2. The lengthy comment that explains how alignment works and why
it's needed is in that function. This is tricky delicate stuff,
so let's put the special cases near the documentation.
} else {
surfwidth = params->depth.mt->logical_width0;
surfheight = params->depth.mt->logical_height0;
--
2.1.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
