[Intel-xe] [PATCH 2/2] drm/xe/uapi: Add IP version and stepping to GT list query

[Lucas De Marchi]

On Wed, Dec 06, 2023 at 12:50:37PM -0800, Matt Roper wrote:
>For modern platforms (MTL and later), both kernel and userspace drivers
>are expected to apply GT programming and workarounds based on the IP
>version and stepping self-reported by the GT hardware via the the GMD_ID
>registers.  Since userspace drivers can't access these registers
>directly, pass along the version and stepping information via the GT
>list query.  Note that the new query fields will remain 0's when running
>on pre-GMD_ID platforms.  Userspace is expected to continue using PCI
>devid / revid on those older platforms.
>
>Although the hardware also has a GMD_ID register for display
>version/stepping, that value is intentionally *not* included anywhere in
>the Xe uapi.  Display userspace should be using platform-agnostic APIs
>and auto-detecting platform capabilities rather than matching specific
>IP versions.
>
>Signed-off-by: Matt Roper <matthew.d.roper at intel.com>
>---
> drivers/gpu/drm/xe/xe_query.c |  8 ++++++++
> include/uapi/drm/xe_drm.h     | 10 +++++++++-
> 2 files changed, 17 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/gpu/drm/xe/xe_query.c b/drivers/gpu/drm/xe/xe_query.c
>index 56d61bf596b2..c3e5163d40da 100644
>--- a/drivers/gpu/drm/xe/xe_query.c
>+++ b/drivers/gpu/drm/xe/xe_query.c
>@@ -12,6 +12,7 @@
> #include <drm/xe_drm.h>
>
> #include "regs/xe_engine_regs.h"
>+#include "regs/xe_gt_regs.h"
> #include "xe_bo.h"
> #include "xe_device.h"
> #include "xe_exec_queue.h"
>@@ -388,6 +389,13 @@ static int query_gt_list(struct xe_device *xe, struct drm_xe_device_query *query
> 				BIT(gt_to_tile(gt)->id) << 1;
> 		gt_list->gt_list[id].far_mem_regions = xe->info.mem_region_mask ^
> 			gt_list->gt_list[id].near_mem_regions;
>+
>+		gt_list->gt_list[id].ip_ver_major =
>+			REG_FIELD_GET(GMD_ID_ARCH_MASK, gt->info.gmdid);
>+		gt_list->gt_list[id].ip_ver_minor =
>+			REG_FIELD_GET(GMD_ID_RELEASE_MASK, gt->info.gmdid);
>+		gt_list->gt_list[id].ip_ver_revid =
>+			REG_FIELD_GET(GMD_ID_REVID, gt->info.gmdid);
> 	}
>
> 	if (copy_to_user(query_ptr, gt_list, size)) {
>diff --git a/include/uapi/drm/xe_drm.h b/include/uapi/drm/xe_drm.h
>index 0895e4d2a981..d84622f54e4b 100644
>--- a/include/uapi/drm/xe_drm.h
>+++ b/include/uapi/drm/xe_drm.h
>@@ -397,8 +397,16 @@ struct drm_xe_gt {
> 	 * memory and memory living in a different tile.
> 	 */
> 	__u64 far_mem_regions;
>+	/** @ip_ver_major: Graphics/media IP major version on GMD_ID platforms */
>+	__u16 ip_ver_major;
>+	/** @ip_ver_major: Graphics/media IP minor version on GMD_ID platforms */
>+	__u16 ip_ver_minor;
>+	/** @ip_ver_major: Graphics/media IP revision ID on GMD_ID platforms */
>+	__u16 ip_ver_revid;

major, minor, rev? why only the last has ID on it?

earlier versions of GMDID from hardware had major/minor/revision naming,
but later started using arch/rel/rev. Not sure why, and it seems to
have similar semantic.  +Rodrigo

IMO it's fine to go with major/minor/rev as it's a more common name to
expose to userspace.

with s/revid/rev/, s/revision ID/revision version/,

	Reviewed-by: Lucas De Marchi <lucas.demarchi at intel.com>

thanks
Lucas De Marchi

>+	/** @pad2: MBZ */
>+	__u16 pad2;
> 	/** @reserved: Reserved */
>-	__u64 reserved[8];
>+	__u64 reserved[7];
> };
>
> /**
>-- 
>2.43.0
>