Re: [PATCH] drm/xe: Add POST_SYNC prefix to post sync instructions

["Bhadane, Dnyaneshwar" <dnyaneshwar.bhadane@intel.com>]

On 24-Mar-26 12:57 AM, Vodapalli, Ravi Kumar wrote:
> 
> On 2/20/2026 1:03 AM, Matthew Brost wrote:
>> On Mon, May 12, 2025 at 06:42:03AM +0000, Bhadane, Dnyaneshwar wrote:
>>>
>>>> -----Original Message-----
>>>> From: Vodapalli, Ravi Kumar <ravi.kumar.vodapalli@intel.com>
>>>> Sent: Thursday, April 24, 2025 3:00 PM
>>>> To: intel-xe@lists.freedesktop.org
>>>> Cc: Vivekanandan, Balasubramani <balasubramani.vivekanandan@intel.com>;
>>>> Roper, Matthew D <matthew.d.roper@intel.com>; De Marchi, Lucas
>>>> <lucas.demarchi@intel.com>; Sousa, Gustavo <gustavo.sousa@intel.com>;
>>>> Taylor, Clinton A <clinton.a.taylor@intel.com>; Atwood, Matthew S
>>>> <matthew.s.atwood@intel.com>; Bhadane, Dnyaneshwar
>>>> <dnyaneshwar.bhadane@intel.com>; Kalvala, Haridhar
>>>> <haridhar.kalvala@intel.com>; Chauhan, Shekhar
>>>> <shekhar.chauhan@intel.com>
>>>> Subject: [PATCH] drm/xe: Add POST_SYNC prefix to post sync instructions
>>>>
>>>> In existing code for PIPE CONTROL post sync operation instructions
>>>> POST_SYNC prefix is not present in the name, add it so that it will 
>>>> represent it
>>>> is a post sync operation type.
>>> Is there a specific reason for this renaming? Is it truly necessary?
>>> Regardless, the terms PIPE_CONTROL_POST_SYNC_TYPE_QW_WRITE and 
>>> PIPE_CONTROL_POST_SYNC_OP_TYPE_QW_WRITE would be more descriptive.
>>> If renaming is indeed required, it should be applied to 
>>> PIPE_CONTROL_WRITE_TIMESTAMP.
>>> This refers to all the post-sync operation types defined and used in 
>>> the code within the Xe/i915 context.
>>>
>>> Renaming can clutter the Git history with unnecessary changes, making 
>>> it harder to track the evolution of the code and understand the 
>>> reasons behind certain modification.
>> I don't think we should consider git history too much in when making
>> choices about renaming. If this is a better name, more inline with bspec
>> I'd say let's use a better name. If we need to rename more operations
>> with post-sync prefixes, let's also do that.
> I have checked bspec only these are the changes with POST_SYNC prefix.

I could see more places this flag is being used. Please make sure it 
reflects all the places.

Dnyaneshwar

>> Matt
>>
>>> Regards,
>>> Dnyaneshwar
>>>> Signed-off-by: Ravi Kumar Vodapalli <ravi.kumar.vodapalli@intel.com>
>>>> ---
>>>>   drivers/gpu/drm/xe/instructions/xe_gpu_commands.h | 2 +-
>>>>   drivers/gpu/drm/xe/xe_ring_ops.c                  | 4 ++--
>>>>   2 files changed, 3 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/xe/instructions/xe_gpu_commands.h
>>>> b/drivers/gpu/drm/xe/instructions/xe_gpu_commands.h
>>>> index 8cfcd3360896..75efdacc4979 100644
>>>> --- a/drivers/gpu/drm/xe/instructions/xe_gpu_commands.h
>>>> +++ b/drivers/gpu/drm/xe/instructions/xe_gpu_commands.h
>>>> @@ -54,7 +54,7 @@
>>>>   #define   PIPE_CONTROL_GLOBAL_SNAPSHOT_RESET        (1<<19)
>>>>   #define      PIPE_CONTROL_TLB_INVALIDATE
>>>>     BIT(18)
>>>>   #define   PIPE_CONTROL_PSD_SYNC                (1<<17)
>>>> -#define   PIPE_CONTROL_QW_WRITE                (1<<14)
>>>> +#define   PIPE_CONTROL_POST_SYNC_QW_WRITE        (1<<14)
>>>>   #define   PIPE_CONTROL_DEPTH_STALL            (1<<13)
>>>>   #define   PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH    (1<<12)
>>>>   #define   PIPE_CONTROL_INSTRUCTION_CACHE_INVALIDATE    (1<<11)
>>>> diff --git a/drivers/gpu/drm/xe/xe_ring_ops.c
>>>> b/drivers/gpu/drm/xe/xe_ring_ops.c
>>>> index a7582b097ae6..da50213b78c5 100644
>>>> --- a/drivers/gpu/drm/xe/xe_ring_ops.c
>>>> +++ b/drivers/gpu/drm/xe/xe_ring_ops.c
>>>> @@ -145,7 +145,7 @@ static int emit_pipe_invalidate(u32 mask_flags, 
>>>> bool
>>>> invalidate_tlb, u32 *dw,
>>>>           PIPE_CONTROL_VF_CACHE_INVALIDATE |
>>>>           PIPE_CONTROL_CONST_CACHE_INVALIDATE |
>>>>           PIPE_CONTROL_STATE_CACHE_INVALIDATE |
>>>> -        PIPE_CONTROL_QW_WRITE |
>>>> +        PIPE_CONTROL_POST_SYNC_QW_WRITE |
>>>>           PIPE_CONTROL_STORE_DATA_INDEX;
>>>>
>>>>       if (invalidate_tlb)
>>>> @@ -216,7 +216,7 @@ static int emit_pipe_imm_ggtt(u32 addr, u32 value,
>>>> bool stall_only, u32 *dw,
>>>>                     int i)
>>>>   {
>>>>       u32 flags = PIPE_CONTROL_CS_STALL |
>>>> PIPE_CONTROL_GLOBAL_GTT_IVB |
>>>> -            PIPE_CONTROL_QW_WRITE;
>>>> +            PIPE_CONTROL_POST_SYNC_QW_WRITE;
>>>>
>>>>       if (!stall_only)
>>>>           flags |= PIPE_CONTROL_FLUSH_ENABLE;
>>>> -- 
>>>> 2.25.1