[PATCH] drm/xe/guc: Promote GUC_GGTT_TOP definition to regs/xe_guc_regs.h

[Matt Roper]

On Wed, Feb 14, 2024 at 06:47:45PM +0100, Michal Wajdeczko wrote:
> 
> 
> On 13.02.2024 23:24, Rodrigo Vivi wrote:
> > On Tue, Feb 13, 2024 at 10:36:34PM +0100, Michal Wajdeczko wrote:
> >> This is a hardware based definition and instead of multiple local
> >> definitions it would be better to have just one common definition.
> >>
> >> Signed-off-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
> >> ---
> >>  drivers/gpu/drm/xe/regs/xe_guc_regs.h | 3 +++
> >>  drivers/gpu/drm/xe/xe_ggtt.c          | 4 +---
> >>  drivers/gpu/drm/xe/xe_guc.c           | 2 --
> >>  3 files changed, 4 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/xe/regs/xe_guc_regs.h b/drivers/gpu/drm/xe/regs/xe_guc_regs.h
> >> index 92320bbc9d3d..087eaa3b4d63 100644
> >> --- a/drivers/gpu/drm/xe/regs/xe_guc_regs.h
> >> +++ b/drivers/gpu/drm/xe/regs/xe_guc_regs.h
> >> @@ -140,4 +140,7 @@ struct guc_doorbell_info {
> >>  	u32 reserved[14];
> >>  } __packed;
> >>  
> >> +/* GuC addresses above GUC_GGTT_TOP also don't map through the GTT */
> >> +#define GUC_GGTT_TOP				0xFEE00000
> >> +
> >>  #endif
> >> diff --git a/drivers/gpu/drm/xe/xe_ggtt.c b/drivers/gpu/drm/xe/xe_ggtt.c
> >> index ab96edb058d6..df5c58c63b74 100644
> >> --- a/drivers/gpu/drm/xe/xe_ggtt.c
> >> +++ b/drivers/gpu/drm/xe/xe_ggtt.c
> >> @@ -11,6 +11,7 @@
> >>  #include <drm/i915_drm.h>
> >>  
> >>  #include "regs/xe_gt_regs.h"
> >> +#include "regs/xe_guc_regs.h"
> > 
> > xe_ggtt component having to access xe_guc_regs doesn't the right movement to me.
> 
> but OTOH it is using xe_wopcm_size() which is also ~ GuC related stuff.
> 
> > 
> > what about moving that to xe_gt_regs?
> 
> maybe new file "regs/xe_ggtt_layout.h" ?
> like we have "regs/xe_lrc_layout.h"
> 
> + Matt

Yeah, if something isn't a regular register offset, but is rather a
constant describing limits or layout, then putting it in a different
header sounds best to me.  If we have (or expect to have) other
constants describing fixed layouts of address spaces, a new dedicated
header for those sounds reasonable.


Matt

> 
> > 
> >>  #include "regs/xe_regs.h"
> >>  #include "xe_assert.h"
> >>  #include "xe_bo.h"
> >> @@ -26,9 +27,6 @@
> >>  #define XELPG_GGTT_PTE_PAT0	BIT_ULL(52)
> >>  #define XELPG_GGTT_PTE_PAT1	BIT_ULL(53)
> >>  
> >> -/* GuC addresses above GUC_GGTT_TOP also don't map through the GTT */
> >> -#define GUC_GGTT_TOP	0xFEE00000
> >> -
> >>  static u64 xelp_ggtt_pte_encode_bo(struct xe_bo *bo, u64 bo_offset,
> >>  				   u16 pat_index)
> >>  {
> >> diff --git a/drivers/gpu/drm/xe/xe_guc.c b/drivers/gpu/drm/xe/xe_guc.c
> >> index 868208a39829..53a72769d3b4 100644
> >> --- a/drivers/gpu/drm/xe/xe_guc.c
> >> +++ b/drivers/gpu/drm/xe/xe_guc.c
> >> @@ -32,8 +32,6 @@
> >>  #include "xe_wa.h"
> >>  #include "xe_wopcm.h"
> >>  
> >> -/* GuC addresses above GUC_GGTT_TOP also don't map through the GTT */
> >> -#define GUC_GGTT_TOP    0xFEE00000
> >>  static u32 guc_bo_ggtt_addr(struct xe_guc *guc,
> >>  			    struct xe_bo *bo)
> >>  {
> >> -- 
> >> 2.43.0
> >>

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation