[Mesa-dev] [Mesa-stable] [PATCH] i965/gen8+: Do full stall when switching pipeline

[Francisco Jerez]

Jason Ekstrand <jason at jlekstrand.net> writes:

> Thanks for tracking this down!
>
> Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>
>
> I'm not sure how we missed this when CS was brought up.  Oh well.
>

I don't think we did.  The hardware docs are somewhat contradictory
about whether this is required:

| Render CS Only: SW must always program PIPE_CONTROL with CS Stall and
| Render Target Cache Flush Enable set prior to programming
| PIPELINE_SELECT command for GPGPU workloads i.e when pipeline mode is
| set to GPGPU. This is required to achieve better GPGPU preemption
| latencies for certain programming sequences. If programming
| PIPE_CONTROL has performance implications then preemption latencies
| can be trade off against performance by not implementing this
| programming note.

This sounds like more of a suggestion (which potentially trades off
throughput for GPGPU latency due to the synchronous execution of the
compute shader) than a requirement, so the stall may not be required and
is likely to impact performance.

I wonder if the following workaround would be as effective fixing the
bug linked below:

 - Speculatively flush the render and depth caches with a pipelined
   PIPE_CONTROL right before PIPELINE_SELECT (note that this still
   allows some parallelism between 3D and GPGPU workloads).

 - On brw_render_cache_set_check_flush(), if the GPGPU pipeline is
   selected, emit an HDC flush+CS stall instead of the depth/render
   target flush+CS stall.

> On Thu, Mar 16, 2017 at 1:24 AM, Topi Pohjolainen <
> topi.pohjolainen at gmail.com> wrote:
>
>> just as earlier gens do.
>>
>> CC:  Jason Ekstrand <jason at jlekstrand.net>
>> Cc: "17.0 13.0" <mesa-stable at lists.freedesktop.org>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96743
>>
>> Signed-off-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
>> ---
>>  src/mesa/drivers/dri/i965/brw_misc_state.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_misc_state.c
>> b/src/mesa/drivers/dri/i965/brw_misc_state.c
>> index 84f0c18..1cf6b04 100644
>> --- a/src/mesa/drivers/dri/i965/brw_misc_state.c
>> +++ b/src/mesa/drivers/dri/i965/brw_misc_state.c
>> @@ -843,8 +843,9 @@ brw_emit_select_pipeline(struct brw_context *brw,
>> enum brw_pipeline pipeline)
>>
>>           brw->ctx.NewDriverState |= BRW_NEW_CC_STATE;
>>        }
>> +   }
>>
>> -   } else if (brw->gen >= 6) {
>> +   if (brw->gen >= 6) {
>>        /* From "BXML » GT » MI » vol1a GPU Overview » [Instruction]
>>         * PIPELINE_SELECT [DevBWR+]":
>>         *
>> --
>> 2.9.3
>>
>>
> _______________________________________________
> mesa-stable mailing list
> mesa-stable at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-stable
-------------- 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/20170316/bcf8a6f5/attachment.sig>