Re: [Intel-gfx] [PATCH 3/3] drm/i915/guc: Use GuC submission API version number

2022-11-22 Thread Ceraolo Spurio, Daniele




On 11/22/2022 12:09 PM, john.c.harri...@intel.com wrote:

From: John Harrison 

The GuC firmware includes an extra version number to specify the
submission API level. So use that rather than the main firmware
version number for submission related checks.

Also, while it is guaranteed that GuC version number components are
only 8-bits in size, other firmwares do not have that restriction. So
stop making assumptions about them generically fitting in a u16
individually, or in a u32 as a combined 8.8.8.

Signed-off-by: John Harrison 
---
  drivers/gpu/drm/i915/gt/uc/intel_guc.h| 11 +++
  .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 15 +--
  drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c  | 91 ---
  drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h  | 10 +-
  drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h  |  3 +-
  5 files changed, 104 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h 
b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
index 1bb3f98292866..bb4dfe707a7d0 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
@@ -158,6 +158,9 @@ struct intel_guc {
bool submission_selected;
/** @submission_initialized: tracks whether GuC submission has been 
initialised */
bool submission_initialized;
+   /** @submission_version: Submission API version of the currently loaded 
firmware */
+   struct intel_uc_fw_ver submission_version;
+
/**
 * @rc_supported: tracks whether we support GuC rc on the current 
platform
 */
@@ -268,6 +271,14 @@ struct intel_guc {
  #endif
  };
  
+/*

+ * GuC version number components are only 8-bit, so converting to a 32bit 8.8.8
+ * integer works.
+ */
+#define MAKE_GUC_VER(maj, min, pat)(((maj) << 16) | ((min) << 8) | (pat))
+#define MAKE_GUC_VER_STRUCT(ver)   MAKE_GUC_VER((ver).major, (ver).minor, 
(ver).patch)
+#define GUC_SUBMIT_VER(guc)
MAKE_GUC_VER_STRUCT((guc)->submission_version)
+
  static inline struct intel_guc *log_to_guc(struct intel_guc_log *log)
  {
return container_of(log, struct intel_guc, log);
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c 
b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index 0a42f1807f52c..53f7f599cde3a 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -1890,7 +1890,7 @@ int intel_guc_submission_init(struct intel_guc *guc)
if (guc->submission_initialized)
return 0;
  
-	if (GET_UC_VER(guc) < MAKE_UC_VER(70, 0, 0)) {

+   if (GUC_SUBMIT_VER(guc) < MAKE_GUC_VER(1, 0, 0)) {
ret = guc_lrc_desc_pool_create_v69(guc);
if (ret)
return ret;
@@ -2330,7 +2330,7 @@ static int register_context(struct intel_context *ce, 
bool loop)
GEM_BUG_ON(intel_context_is_child(ce));
trace_intel_context_register(ce);
  
-	if (GET_UC_VER(guc) >= MAKE_UC_VER(70, 0, 0))

+   if (GUC_SUBMIT_VER(guc) >= MAKE_GUC_VER(1, 0, 0))
ret = register_context_v70(guc, ce, loop);
else
ret = register_context_v69(guc, ce, loop);
@@ -2342,7 +2342,7 @@ static int register_context(struct intel_context *ce, 
bool loop)
set_context_registered(ce);
spin_unlock_irqrestore(&ce->guc_state.lock, flags);
  
-		if (GET_UC_VER(guc) >= MAKE_UC_VER(70, 0, 0))

+   if (GUC_SUBMIT_VER(guc) >= MAKE_GUC_VER(1, 0, 0))
guc_context_policy_init_v70(ce, loop);
}
  
@@ -2956,7 +2956,7 @@ static void __guc_context_set_preemption_timeout(struct intel_guc *guc,

 u16 guc_id,
 u32 preemption_timeout)
  {
-   if (GET_UC_VER(guc) >= MAKE_UC_VER(70, 0, 0)) {
+   if (GUC_SUBMIT_VER(guc) >= MAKE_GUC_VER(1, 0, 0)) {
struct context_policy policy;
  
  		__guc_context_policy_start_klv(&policy, guc_id);

@@ -3283,7 +3283,7 @@ static int guc_context_alloc(struct intel_context *ce)
  static void __guc_context_set_prio(struct intel_guc *guc,
   struct intel_context *ce)
  {
-   if (GET_UC_VER(guc) >= MAKE_UC_VER(70, 0, 0)) {
+   if (GUC_SUBMIT_VER(guc) >= MAKE_GUC_VER(1, 0, 0)) {
struct context_policy policy;
  
  		__guc_context_policy_start_klv(&policy, ce->guc_id.id);

@@ -4366,7 +4366,7 @@ static int guc_init_global_schedule_policy(struct 
intel_guc *guc)
intel_wakeref_t wakeref;
int ret = 0;
  
-	if (GET_UC_VER(guc) < MAKE_UC_VER(70, 3, 0))

+   if (GUC_SUBMIT_VER(guc) < MAKE_GUC_VER(1, 1, 0))
return 0;
  
  	__guc_scheduling_policy_start_klv(&policy);

@@ -4905,6 +4905,9 @@ void intel_guc_submission_print_info(struct intel_guc 
*guc,
if (!sched_engine)
return;
  
+	drm_printf(p, "GuC Submission API Version: %d.%d.%d\n"

[Intel-gfx] [PATCH 3/3] drm/i915/guc: Use GuC submission API version number

2022-11-22 Thread John . C . Harrison
From: John Harrison 

The GuC firmware includes an extra version number to specify the
submission API level. So use that rather than the main firmware
version number for submission related checks.

Also, while it is guaranteed that GuC version number components are
only 8-bits in size, other firmwares do not have that restriction. So
stop making assumptions about them generically fitting in a u16
individually, or in a u32 as a combined 8.8.8.

Signed-off-by: John Harrison 
---
 drivers/gpu/drm/i915/gt/uc/intel_guc.h| 11 +++
 .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 15 +--
 drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c  | 91 ---
 drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h  | 10 +-
 drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h  |  3 +-
 5 files changed, 104 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h 
b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
index 1bb3f98292866..bb4dfe707a7d0 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
@@ -158,6 +158,9 @@ struct intel_guc {
bool submission_selected;
/** @submission_initialized: tracks whether GuC submission has been 
initialised */
bool submission_initialized;
+   /** @submission_version: Submission API version of the currently loaded 
firmware */
+   struct intel_uc_fw_ver submission_version;
+
/**
 * @rc_supported: tracks whether we support GuC rc on the current 
platform
 */
@@ -268,6 +271,14 @@ struct intel_guc {
 #endif
 };
 
+/*
+ * GuC version number components are only 8-bit, so converting to a 32bit 8.8.8
+ * integer works.
+ */
+#define MAKE_GUC_VER(maj, min, pat)(((maj) << 16) | ((min) << 8) | (pat))
+#define MAKE_GUC_VER_STRUCT(ver)   MAKE_GUC_VER((ver).major, (ver).minor, 
(ver).patch)
+#define GUC_SUBMIT_VER(guc)
MAKE_GUC_VER_STRUCT((guc)->submission_version)
+
 static inline struct intel_guc *log_to_guc(struct intel_guc_log *log)
 {
return container_of(log, struct intel_guc, log);
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c 
b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index 0a42f1807f52c..53f7f599cde3a 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -1890,7 +1890,7 @@ int intel_guc_submission_init(struct intel_guc *guc)
if (guc->submission_initialized)
return 0;
 
-   if (GET_UC_VER(guc) < MAKE_UC_VER(70, 0, 0)) {
+   if (GUC_SUBMIT_VER(guc) < MAKE_GUC_VER(1, 0, 0)) {
ret = guc_lrc_desc_pool_create_v69(guc);
if (ret)
return ret;
@@ -2330,7 +2330,7 @@ static int register_context(struct intel_context *ce, 
bool loop)
GEM_BUG_ON(intel_context_is_child(ce));
trace_intel_context_register(ce);
 
-   if (GET_UC_VER(guc) >= MAKE_UC_VER(70, 0, 0))
+   if (GUC_SUBMIT_VER(guc) >= MAKE_GUC_VER(1, 0, 0))
ret = register_context_v70(guc, ce, loop);
else
ret = register_context_v69(guc, ce, loop);
@@ -2342,7 +2342,7 @@ static int register_context(struct intel_context *ce, 
bool loop)
set_context_registered(ce);
spin_unlock_irqrestore(&ce->guc_state.lock, flags);
 
-   if (GET_UC_VER(guc) >= MAKE_UC_VER(70, 0, 0))
+   if (GUC_SUBMIT_VER(guc) >= MAKE_GUC_VER(1, 0, 0))
guc_context_policy_init_v70(ce, loop);
}
 
@@ -2956,7 +2956,7 @@ static void __guc_context_set_preemption_timeout(struct 
intel_guc *guc,
 u16 guc_id,
 u32 preemption_timeout)
 {
-   if (GET_UC_VER(guc) >= MAKE_UC_VER(70, 0, 0)) {
+   if (GUC_SUBMIT_VER(guc) >= MAKE_GUC_VER(1, 0, 0)) {
struct context_policy policy;
 
__guc_context_policy_start_klv(&policy, guc_id);
@@ -3283,7 +3283,7 @@ static int guc_context_alloc(struct intel_context *ce)
 static void __guc_context_set_prio(struct intel_guc *guc,
   struct intel_context *ce)
 {
-   if (GET_UC_VER(guc) >= MAKE_UC_VER(70, 0, 0)) {
+   if (GUC_SUBMIT_VER(guc) >= MAKE_GUC_VER(1, 0, 0)) {
struct context_policy policy;
 
__guc_context_policy_start_klv(&policy, ce->guc_id.id);
@@ -4366,7 +4366,7 @@ static int guc_init_global_schedule_policy(struct 
intel_guc *guc)
intel_wakeref_t wakeref;
int ret = 0;
 
-   if (GET_UC_VER(guc) < MAKE_UC_VER(70, 3, 0))
+   if (GUC_SUBMIT_VER(guc) < MAKE_GUC_VER(1, 1, 0))
return 0;
 
__guc_scheduling_policy_start_klv(&policy);
@@ -4905,6 +4905,9 @@ void intel_guc_submission_print_info(struct intel_guc 
*guc,
if (!sched_engine)
return;
 
+   drm_printf(p, "GuC Submission API Version: %d.%d.%d\n",
+  guc->s