[Mesa-dev] [PATCH 1/9] i965/gen6+: Invalidate constant cache on brw_emit_mi_flush().

[Francisco Jerez]

Kenneth Graunke <kenneth at whitecape.org> writes:

> On Friday, December 9, 2016 11:03:24 AM PST Francisco Jerez wrote:
>> In order to make sure that the constant cache is coherent with
>> previous rendering when we start using it for pull constant loads.
>> ---
>>  src/mesa/drivers/dri/i965/brw_pipe_control.c | 1 +
>>  1 file changed, 1 insertion(+)
>> 
>> diff --git a/src/mesa/drivers/dri/i965/brw_pipe_control.c b/src/mesa/drivers/dri/i965/brw_pipe_control.c
>> index dd426bf..b8f7406 100644
>> --- a/src/mesa/drivers/dri/i965/brw_pipe_control.c
>> +++ b/src/mesa/drivers/dri/i965/brw_pipe_control.c
>> @@ -351,6 +351,7 @@ brw_emit_mi_flush(struct brw_context *brw)
>>        int flags = PIPE_CONTROL_NO_WRITE | PIPE_CONTROL_RENDER_TARGET_FLUSH;
>>        if (brw->gen >= 6) {
>>           flags |= PIPE_CONTROL_INSTRUCTION_INVALIDATE |
>> +                  PIPE_CONTROL_CONST_CACHE_INVALIDATE |
>>                    PIPE_CONTROL_DEPTH_CACHE_FLUSH |
>>                    PIPE_CONTROL_VF_CACHE_INVALIDATE |
>>                    PIPE_CONTROL_TEXTURE_CACHE_INVALIDATE |
>> 
>
> As we've talked about before...brw_emit_mi_flush() basically means
> "I didn't actually think about what kind of flushing I need, so I threw
> random bits at it, and hoped the problems would somehow go away."
>
> I guess we may as well add this to the hodgepodge, since we haven't
> replaced all uses of brw_emit_mi_flush() yet...but I sort of wonder
> whether it does anything useful.
>
> Does anything actually call brw_emit_mi_flush between draws?  (It did
> two years ago, but you and I changed a number of flushes since then...)
>
> The use case I can think of is:
>
> 1. App binds a buffer object as an SSBO, atomic counter buffer,

These (and image stores and atomics) will be handled by
brw_memory_barrier(), which already takes care of invalidating the
constant cache.

>    transform feedback output, pipeline statistics buffer, or whatever

I'm not convinced that coherency is currently managed correctly for
these by the driver (e.g. brw_end_transform_feedback() calls
brw_emit_mi_flush(), and so does hsw_end_transform_feedback(), but only
*before* writing its result into the XFBO, so it won't necessarily be
visible to shaders if the XFBO is immediately reused as e.g. UBO).
Either way the situation should be no worse than using the sampler.

>    (but not rendering - you can't render to a buffer object).

You can indirectly :), by e.g. binding the buffer as PBO and calling
glReadPixels -- Which will call brw_emit_mi_flush() after rendering,
eventually hitting the constant cache invalidation introduced above.

> 2. Drawing or compute dispatch
> 3. App binds that buffer as a UBO.
> 4. More drawing or compute dispatch
>
> It seems like the real solution is to do a CONST_CACHE_INVALIDATE
> when changing UBO bindings.  (That is, assuming the same BO can't
> be bound as a UBO and something else at the same time...)
>
> This isn't a NAK, I'm just wondering if you have any thoughts.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20161213/7c6da4f8/attachment.sig>