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

[Chris Forbes]

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.phogat at gmail.com> wrote:

>
>
> 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/20141107/11c9b72b/attachment.html>