[Mesa-dev] [PATCH] [RFC] i965: Rewrite the HiZ op

[Kenneth Graunke]

On 02/04/2012 09:48 AM, Chad Versace wrote:
> Anyway, that's all moot since that I've killed the shaders. I now just
> call gen6_hiz_init() from inside gen6_hiz_exec().

Awesome.

>> Just put the VS in passthrough mode by removing the GEN6_VS_ENABLE bit from your 3DSTATE_VS packet.  Then, it will do the basic gl_Position = gl_Vertex shader for you without all the hassle of shaders.
>
> This was *much* more difficult than just disabling 3DSTATE_VS.VsEnable. The pass-through clipper on gen6
> expects to fetch some additional vertex attributes, and the vertices in the vbo happened to squash them.
> I finally got this to work by stuffing the attributes into the URB by hacking 3DSTATE_VERTEX_ELEMENTS.

Ah, sorry :(

>>> +   /* To ensure that the batch contains only the resolve, flush the batch
>>> +    * before beginning and after finishing emitting the resolve packets.
>>> +    *
>>> +    * Ideally, we would not need to flush for the resolve op. But, I suspect
>>> +    * that it's unsafe for CMD_PIPELINE_SELECT to occur multiple times in
>>> +    * a single batch, and there is no safe way to ensure that other than by
>>> +    * fencing the resolve with flushes. Ideally, we would just detect if
>>> +    * a batch is in progress and do the right thing, but that would require
>>> +    * the ability to safely manipulate brw_context::state::dirty::brw outside
>>> +    * of brw_state_init().
>>> +    */
>>> +   intel_flush(ctx);
>>> +
>>> +   /* Emit the following packets:
>>> +    *     CMD_PIPELINE_SELECT
>>> +    *     3DSTATE_MULTISAMPLE
>>> +    *     3DSTATE_SAMPLE_MASK
>>> +    *     3DSTATE_GS_SVB_INDEX
>>> +    *     CMD_STATE_SIP
>>> +    *     CMD_VF_STATISTICS
>>> +    */
>>> +   brw_invariant_state.emit(brw);
>>
>> Unnecessary, perhaps even harmful.  The BRW_NEW_CONTEXT dirty bit is already flagged on every new batch, so brw_invariant_state will already be re-emitted.
>
> You're assuming that the hiz op is emitted as a consequence of a draw call already in progress, but that's not guaranteed. So we can't assume that
> brw_invariant_state.emit() has already been called.

Well.  intel_flush causes a new batch to be created.  invariant_state 
_will_ be emitted on every single batch.  I suppose then, the question 
is whether it's been emitted _early_ enough---it's not the top of the 
gen6_atoms list.

Still, I'm having a hard time coming up with a scenario where you'd do a 
HiZ op before invariant_state got emitted.

>> I think intel_flush followed by invarient_state.emit() will actually cause CMD_PIPELINE_SELECT to be emitted twice, which is the exact thing you were trying to avoid.

>> You definitely -do- need to flush /after/ a resolve, because you've just nerfed all the state and it needs to get put back.
>
> Right. That's also why I flag all the dirty bits.

Oh, I missed that on the first read.  Looks good.

>> You won't need this if you drop the shader programs.
>
> I still do, because I have to upload DEPTH_STATE_STATE, whose offset is
> relative to DynamicStateBaseAddress.

You're right of course.

> I wonder if it's safe to set SurfaceStateBaseAddress to null.

It should be.

>> If you drop the VS program, you should be able to safely drop this too.
>
> Nope. The VS URB size must be nonzero to prevent a GPU hang.
> The spec and simulator agree. I'll add a comment quoting the
> spec in the final patch.

Ugh.  Oh well.