[Mesa-dev] [PATCH 7/8] i965/gen7: Enable the features required for GL_ARB_sample_shading
Paul Berry
stereotype441 at gmail.com
Sun Oct 20 16:37:30 CEST 2013
On 16 October 2013 15:20, Ian Romanick <idr at freedesktop.org> wrote:
> On 10/16/2013 02:57 PM, Anuj Phogat wrote:
> > On Tue, Oct 15, 2013 at 3:48 PM, Kenneth Graunke <kenneth at whitecape.org>
> wrote:
> >> On 10/14/2013 10:12 AM, Anuj Phogat wrote:
> >>> - Enable GEN7_WM_MSDISPMODE_PERSAMPLE, GEN7_WM_POSOFFSET_SAMPLE,
> >>> GEN7_WM_OMASK_TO_RENDER_TARGET as per extension's specification.
> >>> - Don't enable GEN7_WM_16_DISPATCH_ENABLE when
> GEN7_WM_MSDISPMODE_PERSAMPLE
> >>> is enabled. Refer IVB PRM Vol. 2, Part 1, Page 288 for details.
> >>>
> >>> Signed-off-by: Anuj Phogat <anuj.phogat at gmail.com>
> >>> ---
> >>> src/mesa/drivers/dri/i965/gen7_wm_state.c | 70
> +++++++++++++++++++++++++++++--
> >>> 1 file changed, 67 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/src/mesa/drivers/dri/i965/gen7_wm_state.c
> b/src/mesa/drivers/dri/i965/gen7_wm_state.c
> >>> index a2046c3..0267e0e 100644
> >>> --- a/src/mesa/drivers/dri/i965/gen7_wm_state.c
> >>> +++ b/src/mesa/drivers/dri/i965/gen7_wm_state.c
> >>> @@ -82,9 +82,15 @@ upload_wm_state(struct brw_context *brw)
> >>> GEN7_WM_BARYCENTRIC_INTERPOLATION_MODE_SHIFT;
> >>>
> >>> /* _NEW_COLOR, _NEW_MULTISAMPLE */
> >>> + /* Enable if the pixel shader kernel generates and outputs oMask.
> >>> + */
> >>> if (fp->program.UsesKill || ctx->Color.AlphaEnabled ||
> >>> - ctx->Multisample.SampleAlphaToCoverage)
> >>> + ctx->Multisample.SampleAlphaToCoverage ||
> >>> + (ctx->Multisample.SampleShading &&
> >>> + (fp->program.Base.OutputsWritten &
> >>> + BITFIELD64_BIT(FRAG_RESULT_SAMPLE_MASK)))) {
> >>> dw1 |= GEN7_WM_KILL_ENABLE;
> >>> + }
> >>>
> >>> /* _NEW_BUFFERS */
> >>> if (brw_color_buffer_write_enabled(brw) || writes_depth ||
> >>> @@ -97,7 +103,25 @@ upload_wm_state(struct brw_context *brw)
> >>> dw1 |= GEN7_WM_MSRAST_ON_PATTERN;
> >>> else
> >>> dw1 |= GEN7_WM_MSRAST_OFF_PIXEL;
> >>> - dw2 |= GEN7_WM_MSDISPMODE_PERPIXEL;
> >>> + /* From arb_sample_shading specification:
> >>> + * "If MULTISAMPLE or SAMPLE_SHADING_ARB is disabled, sample
> shading
> >>> + * has no effect."
> >>> + *
> >>> + * "Using gl_SampleID in a fragment shader causes the entire
> shader
> >>> + * to be evaluated per-sample."
> >>> + * "Using gl_SamplePosition in a fragment shader causes the
> entire
> >>> + * shader to be evaluated per-sample."
> >>> + *
> >>> + * I interprate the above four lines as enable the sample
> shading
> >>> + * if fragment shader uses gl_SampleID or gl_SamplePosition.
> >>> + */
> >>> + if (ctx->Multisample.Enabled &&
> >>> + (ctx->Multisample.SampleShading ||
> >>> + brw->fragment_program->Base.InputsRead &
> VARYING_BIT_SAMPLE_ID ||
> >>> + brw->fragment_program->Base.InputsRead &
> VARYING_BIT_SAMPLE_POS))
> >>> + dw2 |= GEN7_WM_MSDISPMODE_PERSAMPLE;
> >>> + else
> >>> + dw2 |= GEN7_WM_MSDISPMODE_PERPIXEL;
> >>> } else {
> >>> dw1 |= GEN7_WM_MSRAST_OFF_PIXEL;
> >>> dw2 |= GEN7_WM_MSDISPMODE_PERSAMPLE;
> >>> @@ -129,6 +153,8 @@ upload_ps_state(struct brw_context *brw)
> >>> uint32_t dw2, dw4, dw5;
> >>> const int max_threads_shift = brw->is_haswell ?
> >>> HSW_PS_MAX_THREADS_SHIFT : IVB_PS_MAX_THREADS_SHIFT;
> >>> + bool msdispmode_persample = false;
> >>> + bool multisampled_fbo = ctx->DrawBuffer->Visual.samples > 1;
> >>>
> >>> /* BRW_NEW_PS_BINDING_TABLE */
> >>> BEGIN_BATCH(2);
> >>> @@ -169,6 +195,34 @@ upload_ps_state(struct brw_context *brw)
> >>> if (brw->wm.prog_data->nr_params > 0)
> >>> dw4 |= GEN7_PS_PUSH_CONSTANT_ENABLE;
> >>>
> >>> + /* From the IVB PRM, volume 2 part 1, page 287:
> >>> + * "This bit is inserted in the PS payload header and made
> available to
> >>> + * the DataPort (either via the message header or via header
> bypass) to
> >>> + * indicate that oMask data (one or two phases) is included in
> Render
> >>> + * Target Write messages. If present, the oMask data is used to
> mask off
> >>> + * samples."
> >>> + */
> >>> + if (ctx->Extensions.ARB_sample_shading &&
> >>> + (brw->fragment_program->Base.OutputsWritten &
> >>> + BITFIELD64_BIT(FRAG_RESULT_SAMPLE_MASK)))
> >>> + dw4 |= GEN7_PS_OMASK_TO_RENDER_TARGET;
> >>> +
> >>> + /* From the IVB PRM, volume 2 part 1, page 287:
> >>> + * "If the PS kernel does not need the Position XY Offsets to
> >>> + * compute a Position Value, then this field should be programmed
> >>> + * to POSOFFSET_NONE."
> >>> + * "SW Recommendation: If the PS kernel needs the Position Offsets
> >>> + * to compute a Position XY value, this field should match Position
> >>> + * ZW Interpolation Mode to ensure a consistent position.xyzw
> >>> + * computation."
> >>> + * We only require XY sample offsets. So, this recommendation
> doesn't
> >>> + * look useful at the moment. We might need this in future.
> >>> + */
> >>> + if (brw->fragment_program->Base.InputsRead &
> VARYING_BIT_SAMPLE_POS)
> >>> + dw4 |= GEN7_PS_POSOFFSET_SAMPLE;
> >>> + else
> >>> + dw4 |= GEN7_PS_POSOFFSET_NONE;
> >>> +
> >>> /* CACHE_NEW_WM_PROG | _NEW_COLOR
> >>> *
> >>> * The hardware wedges if you have this bit set but don't turn on
> any dual
> >>> @@ -185,7 +239,17 @@ upload_ps_state(struct brw_context *brw)
> >>> dw4 |= GEN7_PS_ATTRIBUTE_ENABLE;
> >>>
> >>> dw4 |= GEN7_PS_8_DISPATCH_ENABLE;
> >>> - if (brw->wm.prog_data->prog_offset_16)
> >>> + msdispmode_persample =
> >>> + ctx->Multisample.Enabled &&
> >>> + (ctx->Multisample.SampleShading ||
> >>> + brw->fragment_program->Base.InputsRead & VARYING_BIT_SAMPLE_ID
> ||
> >>> + brw->fragment_program->Base.InputsRead &
> VARYING_BIT_SAMPLE_POS);
> >>
> >> There are two copies of this expression in the Gen7 patch. You should
> >> probably pull this up earlier, and remove the second copy (like you did
> >> for the SNB code).
> >>
> > These expressions appear in different functions upload_ps_state() and
> > upload_wm_state(). That's the reason I couldn't do what I did for SNB.
>
> So... this code appears in three places? Sounds like it could be a
> helper function. _mesa_is_sample_shading_enabled(ctx, prog).
>
Intel chips only support two modes: per-sample and per-fragment shading.
But ARB_sample_shading is written in such a way that it permits other
possibilities (for instance, some hardware might support 4x MSAA, where
only 2 unique color values and sets of texture coordinates are dispatched
per fragment, and that would be activated by setting
MinSampleShadingARB(0.5)). In order to make this function useful for
implementations that support those other possibilities, I'd recommend that
the function should return the minimum number of fragment shader
invocations that should occur for each fragment.
That is, if the shader uses gl_SampleID or gl_SamplePosition it should
return ctx->DrawBuffer->Visual.samples. Otherwise it should return this
value (from the ARB_sample_shading spec):
max(ceil(MIN_SAMPLE_SHADING_VALUE_ARB*SAMPLES),1)
So maybe we should call it something like
_mesa_get_min_invocations_per_fragment().
Since i965 only supports per-sample and per-fragment dispatch, i965 code
can then do:
(_mesa_get_min_invocations_per_fragment(...) > 1)
to determine whether to do per-fragment or per-sample shading.
>
> >> Otherwise, the same comments for patch 6 apply. This looks good.
> >>
> >> Patch 8 is Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
> >>
> >>> + /* In case of non 1x (i.e 4x, 8x) multisampling with
> MDISPMODE_PERSAMPLE,
> >>> + * only one of SIMD8 and SIMD16 should be enabled.
> >>> + */
> >>> + if (brw->wm.prog_data->prog_offset_16 &&
> >>> + !(multisampled_fbo && msdispmode_persample))
> >>> dw4 |= GEN7_PS_16_DISPATCH_ENABLE;
> >>>
> >>> dw5 |= (brw->wm.prog_data->first_curbe_grf <<
> >>>
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20131020/454d8d4f/attachment-0001.html>
More information about the mesa-dev
mailing list