Re: [PATCH 03/23] drm/xe/xe3p_lpm: Configure MAIN_GAMCTRL_QUEUE_SELECT

[Matt Roper <matthew.d.roper@intel.com>]

On Mon, Oct 13, 2025 at 08:24:35PM -0700, Lucas De Marchi wrote:
> From: Brian Welty <brian.welty@intel.com>
> 
> Starting from Xe3p, there are two different copies of some of the GAM
> registers:  the traditional MCR variant at their old locations, and a
> new unicast copy known as "main_gamctrl."  The Xe driver doesn't use
> these registers directly, but we need to instruct the GuC on which set
> it should use.  Since the new, unicast registers are preferred (since
> they avoid the need for unnecessary MCR synchronization), set a new GuC
> feature flag, GUC_CTL_MAIN_GAMCTRL_QUEUES to convey this decision.  A
> new helper function, xe_guc_using_main_gamctrl_queues(), is added for
> use in the 3 independent places that need to handle configuration of the
> new reporting queues.
> 
> The mmio write to enable the main gamctl is only done during the general
> GuC upload.  The gamctrl registers are not accessed by the GuC during
> hwconfig load.
> 
> Last, the ADS blob for communicating the queue addresses contains both a
> DPA and GGTT offset. The GuC documentation states that DPA is now MBZ
> when using the MAIN_GAMCTRL queues.
> 
> Signed-off-by: Brian Welty <brian.welty@intel.com>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
>  drivers/gpu/drm/xe/regs/xe_gt_regs.h |  3 +++
>  drivers/gpu/drm/xe/xe_gt.h           |  6 ++++++
>  drivers/gpu/drm/xe/xe_guc.c          | 27 +++++++++++++++++++++++++++
>  drivers/gpu/drm/xe/xe_guc.h          |  1 +
>  drivers/gpu/drm/xe/xe_guc_ads.c      |  6 +++++-
>  drivers/gpu/drm/xe/xe_guc_fwif.h     |  1 +
>  6 files changed, 43 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/xe/regs/xe_gt_regs.h b/drivers/gpu/drm/xe/regs/xe_gt_regs.h
> index 51f2a03847f9d..47e13a3fb9072 100644
> --- a/drivers/gpu/drm/xe/regs/xe_gt_regs.h
> +++ b/drivers/gpu/drm/xe/regs/xe_gt_regs.h
> @@ -545,6 +545,9 @@
>  #define SARB_CHICKEN1				XE_REG_MCR(0xe90c)
>  #define   COMP_CKN_IN				REG_GENMASK(30, 29)
>  
> +#define MAIN_GAMCTRL_MODE			XE_REG(0xef00)
> +#define   MAIN_GAMCTRL_QUEUE_SELECT		REG_BIT(0)
> +
>  #define RCU_MODE				XE_REG(0x14800, XE_REG_OPTION_MASKED)
>  #define   RCU_MODE_FIXED_SLICE_CCS_MODE		REG_BIT(1)
>  #define   RCU_MODE_CCS_ENABLE			REG_BIT(0)
> diff --git a/drivers/gpu/drm/xe/xe_gt.h b/drivers/gpu/drm/xe/xe_gt.h
> index 5df2ffe3ff838..9d710049da455 100644
> --- a/drivers/gpu/drm/xe/xe_gt.h
> +++ b/drivers/gpu/drm/xe/xe_gt.h
> @@ -22,6 +22,12 @@
>  
>  #define CCS_MASK(gt) (((gt)->info.engine_mask & XE_HW_ENGINE_CCS_MASK) >> XE_HW_ENGINE_CCS0)
>  
> +#define GT_VER(gt) ({ \
> +	typeof(gt) gt_ = (gt); \
> +	struct xe_device *xe = gt_to_xe(gt_); \
> +	xe_gt_is_media_type(gt_) ? MEDIA_VER(xe) : GRAPHICS_VER(xe); \
> +})
> +
>  extern struct fault_attr gt_reset_failure;
>  static inline bool xe_fault_inject_gt_reset(void)
>  {
> diff --git a/drivers/gpu/drm/xe/xe_guc.c b/drivers/gpu/drm/xe/xe_guc.c
> index d94490979adc0..37e3735f34e63 100644
> --- a/drivers/gpu/drm/xe/xe_guc.c
> +++ b/drivers/gpu/drm/xe/xe_guc.c
> @@ -91,6 +91,9 @@ static u32 guc_ctl_feature_flags(struct xe_guc *guc)
>  	if (xe_configfs_get_psmi_enabled(to_pci_dev(xe->drm.dev)))
>  		flags |= GUC_CTL_ENABLE_PSMI_LOGGING;
>  
> +	if (xe_guc_using_main_gamctrl_queues(guc))
> +		flags |= GUC_CTL_MAIN_GAMCTRL_QUEUES;
> +
>  	return flags;
>  }
>  
> @@ -1255,8 +1258,13 @@ int xe_guc_min_load_for_hwconfig(struct xe_guc *guc)
>  
>  int xe_guc_upload(struct xe_guc *guc)
>  {
> +	struct xe_gt *gt = guc_to_gt(guc);
> +
>  	xe_guc_ads_populate(&guc->ads);
>  
> +	if (xe_guc_using_main_gamctrl_queues(guc))
> +		xe_mmio_write32(&gt->mmio, MAIN_GAMCTRL_MODE, MAIN_GAMCTRL_QUEUE_SELECT);
> +
>  	return __xe_guc_upload(guc);
>  }
>  
> @@ -1657,6 +1665,25 @@ void xe_guc_declare_wedged(struct xe_guc *guc)
>  	xe_guc_submit_wedge(guc);
>  }
>  
> +/**
> + * xe_guc_using_main_gamctrl_queues() - Detect which reporting queues to use.
> + * @guc: The GuC object
> + *
> + * For Xe3p and beyond, we want to program the hardware to use the
> + * "Main GAMCTRL queue" rather than the legacy queue before we upload
> + * the GuC firmware.  This will allow the GuC to use a new set of
> + * registers for pagefault handling and avoid some unnecessary
> + * complications with MCR register range handling.
> + *
> + * Return: true if can use new main gamctrl queues.
> + */
> +bool xe_guc_using_main_gamctrl_queues(struct xe_guc *guc)
> +{
> +	struct xe_gt *gt = guc_to_gt(guc);
> +
> +	return GT_VER(gt) >= 35;

Revisiting the spec on this, I'm not sure whether using GT_VER() here is
actually the right thing to do.  As far as I can see, the media GT does
not actually have a "main gamctrl" register range at all (i.e., bspec
76445 only lists MCR gamctrl ranges, and 0x38EF00-0x38EFFF falls within
a reserved/unused block).  That means that registers like
MAIN_GAMCTRL_MODE that we're trying to write during xe_guc_upload above
don't actually exist for the media GT.  Furthermore, the tagging on the
register detail page 73540 also seems to imply that this only applies to
the primary/graphics GT.


Matt

> +}
> +
>  #if IS_ENABLED(CONFIG_DRM_XE_KUNIT_TEST)
>  #include "tests/xe_guc_g2g_test.c"
>  #endif
> diff --git a/drivers/gpu/drm/xe/xe_guc.h b/drivers/gpu/drm/xe/xe_guc.h
> index 1cca05967e621..e2d4c5f44ae34 100644
> --- a/drivers/gpu/drm/xe/xe_guc.h
> +++ b/drivers/gpu/drm/xe/xe_guc.h
> @@ -52,6 +52,7 @@ void xe_guc_stop_prepare(struct xe_guc *guc);
>  void xe_guc_stop(struct xe_guc *guc);
>  int xe_guc_start(struct xe_guc *guc);
>  void xe_guc_declare_wedged(struct xe_guc *guc);
> +bool xe_guc_using_main_gamctrl_queues(struct xe_guc *guc);
>  
>  #if IS_ENABLED(CONFIG_DRM_XE_KUNIT_TEST)
>  int xe_guc_g2g_test_notification(struct xe_guc *guc, u32 *payload, u32 len);
> diff --git a/drivers/gpu/drm/xe/xe_guc_ads.c b/drivers/gpu/drm/xe/xe_guc_ads.c
> index 22ac2a8b74c80..bcb85a1bf26d9 100644
> --- a/drivers/gpu/drm/xe/xe_guc_ads.c
> +++ b/drivers/gpu/drm/xe/xe_guc_ads.c
> @@ -820,16 +820,20 @@ static void guc_mmio_reg_state_init(struct xe_guc_ads *ads)
>  static void guc_um_init_params(struct xe_guc_ads *ads)
>  {
>  	u32 um_queue_offset = guc_ads_um_queues_offset(ads);
> +	struct xe_guc *guc = ads_to_guc(ads);
>  	u64 base_dpa;
>  	u32 base_ggtt;
> +	bool with_dpa;
>  	int i;
>  
> +	with_dpa = !xe_guc_using_main_gamctrl_queues(guc);
> +
>  	base_ggtt = xe_bo_ggtt_addr(ads->bo) + um_queue_offset;
>  	base_dpa = xe_bo_main_addr(ads->bo, PAGE_SIZE) + um_queue_offset;
>  
>  	for (i = 0; i < GUC_UM_HW_QUEUE_MAX; ++i) {
>  		ads_blob_write(ads, um_init_params.queue_params[i].base_dpa,
> -			       base_dpa + (i * GUC_UM_QUEUE_SIZE));
> +			       with_dpa ? (base_dpa + (i * GUC_UM_QUEUE_SIZE)) : 0);
>  		ads_blob_write(ads, um_init_params.queue_params[i].base_ggtt_address,
>  			       base_ggtt + (i * GUC_UM_QUEUE_SIZE));
>  		ads_blob_write(ads, um_init_params.queue_params[i].size_in_bytes,
> diff --git a/drivers/gpu/drm/xe/xe_guc_fwif.h b/drivers/gpu/drm/xe/xe_guc_fwif.h
> index 50c4c2406132e..c90dd266e9cf9 100644
> --- a/drivers/gpu/drm/xe/xe_guc_fwif.h
> +++ b/drivers/gpu/drm/xe/xe_guc_fwif.h
> @@ -113,6 +113,7 @@ struct guc_update_exec_queue_policy {
>  #define   GUC_CTL_ENABLE_SLPC		BIT(2)
>  #define   GUC_CTL_ENABLE_LITE_RESTORE	BIT(4)
>  #define   GUC_CTL_ENABLE_PSMI_LOGGING	BIT(7)
> +#define   GUC_CTL_MAIN_GAMCTRL_QUEUES	BIT(9)
>  #define   GUC_CTL_DISABLE_SCHEDULER	BIT(14)
>  
>  #define GUC_CTL_DEBUG			3
> 
> -- 
> 2.51.0
> 

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