[Intel-xe] Render cache flushes WAS drm/xe: Add missing TLB invalidation to emit_pipe_invalidate()

[Thomas Hellström]

Changing subject to avoid mixing it up with the potentially missing TLB 
flushes + adding MattB and MattR for additional input,

On 5/31/23 21:12, Souza, Jose wrote:
...
>
>>> Do you see any PIPE_CONTROL flush at the end of batch buffers that i915 does but Xe don't?
>> The emit_fini_breadcrumb() called from __i915_request_submit() indeed
>> seems to emit the flushes needed, whereas the corresponding
>> emit_pipe_imm_ggtt() in xe doesn't.
> Huum!
> What do you think? Xe should do it like i915 or UMDs should do it?

I would be inclined to favor UMDs should do this to avoid excessive 
unnecessary flushing, but then we'd encounter a problem in that the KMD 
needs to be able to ensure that a flush has been performed before 
swapping out / evicting data or freeing a used bo, So I'd say we *need* 
Xe to do it. If this in the future turns out to be a performance 
problem, there are ways to only trigger flushes when somebody starts 
waiting for a fence.

Anybody with another opinion here?

/Thomas


>
>> /Thomas
>>
>>
>>>> /Thomas
>>>>
>>>>> /Thomas
>>>>>
>>>>>>> Thanks,
>>>>>>> Thomas
>>>>>>>
>>>>>>>
>>>>>>>> Signed-off-by: José Roberto de Souza <jose.souza at intel.com>
>>>>>>>> ---
>>>>>>>>     drivers/gpu/drm/xe/regs/xe_gpu_commands.h | 1 +
>>>>>>>>     drivers/gpu/drm/xe/xe_ring_ops.c          | 6 ++++--
>>>>>>>>     2 files changed, 5 insertions(+), 2 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/drivers/gpu/drm/xe/regs/xe_gpu_commands.h
>>>>>>>> b/drivers/gpu/drm/xe/regs/xe_gpu_commands.h
>>>>>>>> index 0f9c5b0b8a3ba..7c7320efea739 100644
>>>>>>>> --- a/drivers/gpu/drm/xe/regs/xe_gpu_commands.h
>>>>>>>> +++ b/drivers/gpu/drm/xe/regs/xe_gpu_commands.h
>>>>>>>> @@ -73,6 +73,7 @@
>>>>>>>>     #define
>>>>>>>> PIPE_CONTROL_STORE_DATA_INDEX                        (1<<21)
>>>>>>>>     #define
>>>>>>>> PIPE_CONTROL_CS_STALL                                (1<<20)
>>>>>>>>     #define PIPE_CONTROL_GLOBAL_SNAPSHOT_RESET           (1<<19)
>>>>>>>> +#define PIPE_CONTROL_TLB_INVALIDATE                  (1<<18)
>>>>>>>>     #define
>>>>>>>> PIPE_CONTROL_PSD_SYNC                                (1<<17)
>>>>>>>>     #define
>>>>>>>> PIPE_CONTROL_QW_WRITE                                (1<<14)
>>>>>>>>     #define PIPE_CONTROL_DEPTH_STALL                     (1<<13)
>>>>>>>> diff --git a/drivers/gpu/drm/xe/xe_ring_ops.c
>>>>>>>> b/drivers/gpu/drm/xe/xe_ring_ops.c
>>>>>>>> index d2fa0b4c8bcc0..4f3ef39109b9b 100644
>>>>>>>> --- a/drivers/gpu/drm/xe/xe_ring_ops.c
>>>>>>>> +++ b/drivers/gpu/drm/xe/xe_ring_ops.c
>>>>>>>> @@ -37,7 +37,8 @@
>>>>>>>>                    PIPE_CONTROL_INDIRECT_STATE_DISABLE | \
>>>>>>>>                    PIPE_CONTROL_FLUSH_ENABLE | \
>>>>>>>>                    PIPE_CONTROL_TEXTURE_CACHE_INVALIDATE | \
>>>>>>>> -               PIPE_CONTROL_DC_FLUSH_ENABLE)
>>>>>>>> +               PIPE_CONTROL_DC_FLUSH_ENABLE | \
>>>>>>>> +               PIPE_CONTROL_TLB_INVALIDATE)
>>>>>>>>       static u32 preparser_disable(bool state)
>>>>>>>>     {
>>>>>>>> @@ -117,7 +118,8 @@ static int emit_pipe_invalidate(u32 mask_flags,
>>>>>>>> u32 *dw, int i)
>>>>>>>>                    PIPE_CONTROL_CONST_CACHE_INVALIDATE |
>>>>>>>>                    PIPE_CONTROL_STATE_CACHE_INVALIDATE |
>>>>>>>>                    PIPE_CONTROL_QW_WRITE |
>>>>>>>> -               PIPE_CONTROL_STORE_DATA_INDEX;
>>>>>>>> +               PIPE_CONTROL_STORE_DATA_INDEX |
>>>>>>>> +               PIPE_CONTROL_TLB_INVALIDATE;
>>>>>>>>              flags &= ~mask_flags;