[Mesa-stable] [Mesa-dev] [PATCH] i965: Select pipeline and emit state base address in Gen8+ HiZ ops.

[Nanley Chery]

On Mon, Feb 06, 2017 at 03:55:49PM -0800, Kenneth Graunke wrote:
> If a HiZ op is the first thing in the batch, we should make sure
> to select the render pipeline and emit state base address before
> proceeding.
> 
> I believe 3DSTATE_WM_HZ_OP creates 3DPRIMITIVEs internally, and
> dispatching those on the GPGPU pipeline seems a bit sketchy.  I'm

Yes, it does seem like we currently allow HZ_OPs within a GPGPU
pipeline. This patch should fix that problem.

> not actually sure that STATE_BASE_ADDRESS is necessary, as the
> depth related commands use graphics addresses, not ones relative
> to the base address...but we're likely to set it as part of the
> next operation anyway, so we should just do it right away.
> 

I agree, re-emitting STATE_BASE_ADDRESS doesn't seem necessary. I think
we should drop this part of the patch and add it back in later if we get
some data that it's necessary. Leaving it there may be distracting to
some readers and the BDW PRM warns that it's an expensive command:

	Execution of this command causes a full pipeline flush, thus its
	use should be minimized for higher performance.

> Cc: "17.0" <mesa-stable at lists.freedesktop.org>
> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> ---
>  src/mesa/drivers/dri/i965/gen8_depth_state.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/src/mesa/drivers/dri/i965/gen8_depth_state.c b/src/mesa/drivers/dri/i965/gen8_depth_state.c
> index a7e61354fd5..620b32df8bb 100644
> --- a/src/mesa/drivers/dri/i965/gen8_depth_state.c
> +++ b/src/mesa/drivers/dri/i965/gen8_depth_state.c
> @@ -404,6 +404,9 @@ gen8_hiz_exec(struct brw_context *brw, struct intel_mipmap_tree *mt,
>     if (op == BLORP_HIZ_OP_NONE)
>        return;
>  

It would be helpful if you included the rationale here as a code
comment. Something like the first two sentences of your commit message
should work.

> +   brw_select_pipeline(brw, BRW_RENDER_PIPELINE);

According to Vol07 of the BDW+ PRMs,

	The previously active pipeline needs to be flushed via the
	MI_FLUSH command immediately before switching to a different
	pipeline via use of the PIPELINE_SELECT command.

However it doesn't look like MI_FLUSH is present after HSW. So there
shouldn't be any additional work to do here.

-Nanley

> +   brw_upload_state_base_address(brw);
> +
>     /* Disable the PMA stall fix since we're about to do a HiZ operation. */
>     if (brw->gen == 8)
>        gen8_write_pma_stall_bits(brw, 0);
> -- 
> 2.11.0
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev