[PATCH 1/2] drm/xe/guc: Clean up of GuC 'CTL' defines
Cavitt, Jonathan
jonathan.cavitt at intel.com
Wed Jun 11 22:04:52 UTC 2025
-----Original Message-----
From: Intel-xe <intel-xe-bounces at lists.freedesktop.org> On Behalf Of John.C.Harrison at Intel.com
Sent: Wednesday, June 11, 2025 2:06 PM
To: Intel-Xe at Lists.FreeDesktop.Org
Cc: Harrison, John C <john.c.harrison at intel.com>
Subject: [PATCH 1/2] drm/xe/guc: Clean up of GuC 'CTL' defines
>
> From: John Harrison <John.C.Harrison at Intel.com>
>
> All the field generation for the CTL defines (used for GuC init data)
> were hand-rolled rather than using FIELD_PREP/REG_GENMASK/BIT macros.
>
> Also, there were a bunch of macros defined for verbosity settings that
> were never used - it's just a number from 0 to 3, why do we need a define
> for each level!?
LGTM, though we can probably remove everything after the hyphen in this
sentence, as the contents past it might come across as incredulous or accusatory.
But besides that:
Reviewed-by: Jonathan Cavitt <jonathan.cavitt at intel.com>
-Jonathan Cavitt
>
> So fix that all up.
>
> Signed-off-by: John Harrison <John.C.Harrison at Intel.com>
> ---
> drivers/gpu/drm/xe/xe_guc.c | 21 ++++++---------------
> drivers/gpu/drm/xe/xe_guc_fwif.h | 28 +++++++++-------------------
> 2 files changed, 15 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_guc.c b/drivers/gpu/drm/xe/xe_guc.c
> index 0fcdf389711a..e16d19b44bcc 100644
> --- a/drivers/gpu/drm/xe/xe_guc.c
> +++ b/drivers/gpu/drm/xe/xe_guc.c
> @@ -72,8 +72,7 @@ static u32 guc_ctl_debug_flags(struct xe_guc *guc)
> if (!GUC_LOG_LEVEL_IS_VERBOSE(level))
> flags |= GUC_LOG_DISABLED;
> else
> - flags |= GUC_LOG_LEVEL_TO_VERBOSITY(level) <<
> - GUC_LOG_VERBOSITY_SHIFT;
> + flags |= FIELD_PREP(GUC_LOG_VERBOSITY, GUC_LOG_LEVEL_TO_VERBOSITY(level));
>
> return flags;
> }
> @@ -116,22 +115,14 @@ static u32 guc_ctl_log_params_flags(struct xe_guc *guc)
> BUILD_BUG_ON(!CAPTURE_BUFFER_SIZE);
> BUILD_BUG_ON(!IS_ALIGNED(CAPTURE_BUFFER_SIZE, CAPTURE_UNIT));
>
> - BUILD_BUG_ON((CRASH_BUFFER_SIZE / LOG_UNIT - 1) >
> - (GUC_LOG_CRASH_MASK >> GUC_LOG_CRASH_SHIFT));
> - BUILD_BUG_ON((DEBUG_BUFFER_SIZE / LOG_UNIT - 1) >
> - (GUC_LOG_DEBUG_MASK >> GUC_LOG_DEBUG_SHIFT));
> - BUILD_BUG_ON((CAPTURE_BUFFER_SIZE / CAPTURE_UNIT - 1) >
> - (GUC_LOG_CAPTURE_MASK >> GUC_LOG_CAPTURE_SHIFT));
> -
> flags = GUC_LOG_VALID |
> GUC_LOG_NOTIFY_ON_HALF_FULL |
> CAPTURE_FLAG |
> LOG_FLAG |
> - ((CRASH_BUFFER_SIZE / LOG_UNIT - 1) << GUC_LOG_CRASH_SHIFT) |
> - ((DEBUG_BUFFER_SIZE / LOG_UNIT - 1) << GUC_LOG_DEBUG_SHIFT) |
> - ((CAPTURE_BUFFER_SIZE / CAPTURE_UNIT - 1) <<
> - GUC_LOG_CAPTURE_SHIFT) |
> - (offset << GUC_LOG_BUF_ADDR_SHIFT);
> + FIELD_PREP(GUC_LOG_CRASH, CRASH_BUFFER_SIZE / LOG_UNIT - 1) |
> + FIELD_PREP(GUC_LOG_DEBUG, DEBUG_BUFFER_SIZE / LOG_UNIT - 1) |
> + FIELD_PREP(GUC_LOG_CAPTURE, CAPTURE_BUFFER_SIZE / CAPTURE_UNIT - 1) |
> + FIELD_PREP(GUC_LOG_BUF_ADDR, offset);
>
> #undef LOG_UNIT
> #undef LOG_FLAG
> @@ -144,7 +135,7 @@ static u32 guc_ctl_log_params_flags(struct xe_guc *guc)
> static u32 guc_ctl_ads_flags(struct xe_guc *guc)
> {
> u32 ads = guc_bo_ggtt_addr(guc, guc->ads.bo) >> PAGE_SHIFT;
> - u32 flags = ads << GUC_ADS_ADDR_SHIFT;
> + u32 flags = FIELD_PREP(GUC_ADS_ADDR, ads);
>
> return flags;
> }
> diff --git a/drivers/gpu/drm/xe/xe_guc_fwif.h b/drivers/gpu/drm/xe/xe_guc_fwif.h
> index 6f57578b07cb..b05646cb4fb1 100644
> --- a/drivers/gpu/drm/xe/xe_guc_fwif.h
> +++ b/drivers/gpu/drm/xe/xe_guc_fwif.h
> @@ -84,13 +84,10 @@ struct guc_update_exec_queue_policy {
> #define GUC_LOG_NOTIFY_ON_HALF_FULL BIT(1)
> #define GUC_LOG_CAPTURE_ALLOC_UNITS BIT(2)
> #define GUC_LOG_LOG_ALLOC_UNITS BIT(3)
> -#define GUC_LOG_CRASH_SHIFT 4
> -#define GUC_LOG_CRASH_MASK (0x3 << GUC_LOG_CRASH_SHIFT)
> -#define GUC_LOG_DEBUG_SHIFT 6
> -#define GUC_LOG_DEBUG_MASK (0xF << GUC_LOG_DEBUG_SHIFT)
> -#define GUC_LOG_CAPTURE_SHIFT 10
> -#define GUC_LOG_CAPTURE_MASK (0x3 << GUC_LOG_CAPTURE_SHIFT)
> -#define GUC_LOG_BUF_ADDR_SHIFT 12
> +#define GUC_LOG_CRASH REG_GENMASK(5, 4)
> +#define GUC_LOG_DEBUG REG_GENMASK(9, 6)
> +#define GUC_LOG_CAPTURE REG_GENMASK(11, 10)
> +#define GUC_LOG_BUF_ADDR REG_GENMASK(31, 12)
>
> #define GUC_CTL_WA 1
> #define GUC_WA_GAM_CREDITS BIT(10)
> @@ -110,21 +107,14 @@ struct guc_update_exec_queue_policy {
> #define GUC_CTL_DISABLE_SCHEDULER BIT(14)
>
> #define GUC_CTL_DEBUG 3
> -#define GUC_LOG_VERBOSITY_SHIFT 0
> -#define GUC_LOG_VERBOSITY_LOW (0 << GUC_LOG_VERBOSITY_SHIFT)
> -#define GUC_LOG_VERBOSITY_MED (1 << GUC_LOG_VERBOSITY_SHIFT)
> -#define GUC_LOG_VERBOSITY_HIGH (2 << GUC_LOG_VERBOSITY_SHIFT)
> -#define GUC_LOG_VERBOSITY_ULTRA (3 << GUC_LOG_VERBOSITY_SHIFT)
> -#define GUC_LOG_VERBOSITY_MIN 0
> +#define GUC_LOG_VERBOSITY REG_GENMASK(1, 0)
> #define GUC_LOG_VERBOSITY_MAX 3
> -#define GUC_LOG_VERBOSITY_MASK 0x0000000f
> -#define GUC_LOG_DESTINATION_MASK (3 << 4)
> -#define GUC_LOG_DISABLED (1 << 6)
> -#define GUC_PROFILE_ENABLED (1 << 7)
> +#define GUC_LOG_DESTINATION REG_GENMASK(5, 4)
> +#define GUC_LOG_DISABLED BIT(6)
> +#define GUC_PROFILE_ENABLED BIT(7)
>
> #define GUC_CTL_ADS 4
> -#define GUC_ADS_ADDR_SHIFT 1
> -#define GUC_ADS_ADDR_MASK (0xFFFFF << GUC_ADS_ADDR_SHIFT)
> +#define GUC_ADS_ADDR REG_GENMASK(21, 1)
>
> #define GUC_CTL_DEVID 5
>
> --
> 2.49.0
>
>
More information about the Intel-xe
mailing list