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" <anuj.pho...@gmail.com> wrote:
>
>
> On Tue, Nov 4, 2014 at 8:40 AM, Chris Forbes <chr...@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 <chr...@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@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.pho...@gmail.com>
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
