-----Original Message----- From: Wajdeczko, Michal <[email protected]> Sent: Wednesday, May 7, 2025 9:34 AM To: Hirschfeld, Dafna <[email protected]>; Cavitt, Jonathan <[email protected]> Cc: [email protected]; Gupta, saurabhg <[email protected]>; Zuo, Alex <[email protected]>; [email protected]; Brost, Matthew <[email protected]>; Zhang, Jianxun <[email protected]>; Lin, Shuicheng <[email protected]>; [email protected]; Mrozek, Michal <[email protected]>; Jadav, Raag <[email protected]>; Harrison, John C <[email protected]>; Briano, Ivan <[email protected]>; Auld, Matthew <[email protected]> Subject: Re: [PATCH v23 2/5] drm/xe/xe_gt_pagefault: Move pagefault struct to header > > On 29.04.2025 16:22, Dafna Hirschfeld wrote: > > On 24.04.2025 20:49, Jonathan Cavitt wrote: > >> Move the pagefault struct from xe_gt_pagefault.c to the > >> xe_gt_pagefault_types.h header file, and move the associated enum values > >> into the regs folder under xe_pagefault_desc.h > >> > >> Since xe_pagefault_desc.h is being initialized here, also move the > >> xe_guc_pagefault_desc hardware formats to the new file. > >> > >> v2: > >> - Normalize names for common header (Matt Brost) > >> > >> v3: > >> - s/Migrate/Move (Michal W) > >> - s/xe_pagefault/xe_gt_pagefault (Michal W) > >> - Create new header file, xe_gt_pagefault_types.h (Michal W) > >> - Add kernel docs (Michal W) > >> > >> v4: > >> - Fix includes usage (Michal W) > >> - Reference Bspec (Michal W) > >> > >> v5: > >> - Convert enums to defines in regs folder (Michal W) > >> - Move xe_guc_pagefault_desc to regs folder (Michal W) > >> > >> Bspec: 77412 > > maybe also mention 59654 here? > > >> Signed-off-by: Jonathan Cavitt <[email protected]> > >> Reviewed-by: Shuicheng Lin <[email protected]> > >> Acked-by: Matthew Brost <[email protected]> > >> Cc: Michal Wajdeczko <[email protected]> > >> --- > >> drivers/gpu/drm/xe/regs/xe_pagefault_desc.h | 49 +++++++++++++++++++++ > >> drivers/gpu/drm/xe/xe_gt_pagefault.c | 43 ++++-------------- > >> drivers/gpu/drm/xe/xe_gt_pagefault_types.h | 42 ++++++++++++++++++ > >> drivers/gpu/drm/xe/xe_guc_fwif.h | 28 ------------ > >> 4 files changed, 100 insertions(+), 62 deletions(-) > >> create mode 100644 drivers/gpu/drm/xe/regs/xe_pagefault_desc.h > >> create mode 100644 drivers/gpu/drm/xe/xe_gt_pagefault_types.h > >> > >> diff --git a/drivers/gpu/drm/xe/regs/xe_pagefault_desc.h b/drivers/ > >> gpu/drm/xe/regs/xe_pagefault_desc.h > >> new file mode 100644 > >> index 000000000000..a169ac274e14 > >> --- /dev/null > >> +++ b/drivers/gpu/drm/xe/regs/xe_pagefault_desc.h > > > > Maybe change the file name to xe_guc_pagefault_desc.h , > > since this is currently guc specific. > > IMO 'guc' tag is not applicable here > > my understanding was that GuC sends data as it was read from the HW > regs, so the origin of the struct layout is HW, hence we put it in regs/ > where we have all the HW defs, not in the abi/ folder which is for FW defs. > > unless we want to make this struct part of the stable GuC pagefault ABI, > and then even if HW definition/layout would change, we will stick with > current layout. Then agree, we should move all this to abi/ and also > drop the Bspec reference as n/a
So, to summarize, you want me to *not* change the name of the regs/xe_pagefault_desc.h file? I'll get on reverting that change shortly. > > > > > Also, the define 'PF_MSG_LEN_DW 4' relates to the > > length of this struct so should move here. > > any G2H message related definitions should be in the abi/ GuC files, not > here in regs/ where we keep HW definitions. > > and please don't start define name with "PF", use "GUC" instead > > and btw, in many places by message length we assume also HXG header > length, so the actual length of the G2H 6002 message is 5 as it includes > 1 DW of header with DATA0 and 4 DWs of payload with DATA1..4 > > #define GUC2HOST_NOTIFY_PAGE_FAULT_REQ_DESC_MSG_LEN \ > (GUC_HXG_REQUEST_MSG_MIN_LEN + 4u) Should I replace all instances of PF_MSG_LEN_DW with GUC2HOST_NOTIFY_PAGE_FAULT_REQ_DESC_MSG_LEN, instead of attempting to relocate and rename the value? If not, which abi file should I relocate the value to, and what should the value be renamed to? GUC_PF_MSG_LEN_DW? -Jonathan Cavitt > > > > > Thanks, > > Dafna > > >
