Re: [Intel-gfx] [PATCH 3/4] drm/i915/guc: Add support for w/a KLVs

2023-10-27 Thread John Harrison 

On 10/6/2023 17:38, Belgaumkar, Vinay wrote:

On 9/15/2023 2:55 PM, john.c.harri...@intel.com wrote:

From: John Harrison 

To prevent running out of bits, new w/a enable flags are being added
via a KLV system instead of a 32 bit flags word.

Signed-off-by: John Harrison 
---
  .../gpu/drm/i915/gt/uc/abi/guc_errors_abi.h   |  1 +
  drivers/gpu/drm/i915/gt/uc/intel_guc.h    |  3 +
  drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c    | 64 ++-
  drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c |  6 ++
  drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h   |  5 +-
  5 files changed, 77 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_errors_abi.h 
b/drivers/gpu/drm/i915/gt/uc/abi/guc_errors_abi.h

index dabeaf4f245f3..00d6402333f8e 100644
--- a/drivers/gpu/drm/i915/gt/uc/abi/guc_errors_abi.h
+++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_errors_abi.h
@@ -36,6 +36,7 @@ enum intel_guc_load_status {
  INTEL_GUC_LOAD_STATUS_INVALID_INIT_DATA_RANGE_START,
  INTEL_GUC_LOAD_STATUS_MPU_DATA_INVALID = 0x73,
  INTEL_GUC_LOAD_STATUS_INIT_MMIO_SAVE_RESTORE_INVALID   = 0x74,
+    INTEL_GUC_LOAD_STATUS_KLV_WORKAROUND_INIT_ERROR    = 0x75,
  INTEL_GUC_LOAD_STATUS_INVALID_INIT_DATA_RANGE_END,
    INTEL_GUC_LOAD_STATUS_READY    = 0xF0,
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h 
b/drivers/gpu/drm/i915/gt/uc/intel_guc.h

index 6c392bad29c19..3b1fc5f96306b 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
@@ -186,6 +186,8 @@ struct intel_guc {
  struct guc_mmio_reg *ads_regset;
  /** @ads_golden_ctxt_size: size of the golden contexts in the 
ADS */

  u32 ads_golden_ctxt_size;
+    /** @ads_waklv_size: size of workaround KLVs */
+    u32 ads_waklv_size;
  /** @ads_capture_size: size of register lists in the ADS used 
for error capture */

  u32 ads_capture_size;
  /** @ads_engine_usage_size: size of engine usage in the ADS */
@@ -295,6 +297,7 @@ struct intel_guc {
  #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)
+#define GUC_FIRMWARE_VER(guc) 
MAKE_GUC_VER_STRUCT((guc)->fw.file_selected.ver)
    static inline struct intel_guc *log_to_guc(struct intel_guc_log 
*log)

  {
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c 
b/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c

index 63724e17829a7..792910af3a481 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c
@@ -46,6 +46,10 @@
   *  +---+
   *  | padding   |
   *  +---+ <== 4K aligned
+ *  | w/a KLVs  |
+ *  +---+
+ *  | padding   |
+ *  +---+ <== 4K aligned
   *  | capture lists |
   *  +---+
   *  | padding   |
@@ -88,6 +92,11 @@ static u32 guc_ads_golden_ctxt_size(struct 
intel_guc *guc)

  return PAGE_ALIGN(guc->ads_golden_ctxt_size);
  }
  +static u32 guc_ads_waklv_size(struct intel_guc *guc)
+{
+    return PAGE_ALIGN(guc->ads_waklv_size);
+}
+
  static u32 guc_ads_capture_size(struct intel_guc *guc)
  {
  return PAGE_ALIGN(guc->ads_capture_size);
@@ -113,7 +122,7 @@ static u32 guc_ads_golden_ctxt_offset(struct 
intel_guc *guc)

  return PAGE_ALIGN(offset);
  }
  -static u32 guc_ads_capture_offset(struct intel_guc *guc)
+static u32 guc_ads_waklv_offset(struct intel_guc *guc)
  {
  u32 offset;
  @@ -123,6 +132,16 @@ static u32 guc_ads_capture_offset(struct 
intel_guc *guc)

  return PAGE_ALIGN(offset);
  }
  +static u32 guc_ads_capture_offset(struct intel_guc *guc)
+{
+    u32 offset;
+
+    offset = guc_ads_waklv_offset(guc) +
+ guc_ads_waklv_size(guc);
+
+    return PAGE_ALIGN(offset);
+}
+
  static u32 guc_ads_private_data_offset(struct intel_guc *guc)
  {
  u32 offset;
@@ -791,6 +810,40 @@ guc_capture_prep_lists(struct intel_guc *guc)
  return PAGE_ALIGN(total_size);
  }
  +static void guc_waklv_init(struct intel_guc *guc)
+{
+    struct intel_gt *gt = guc_to_gt(guc);
+    u32 offset, addr_ggtt, remain, size;
+
+    if (!intel_uc_uses_guc_submission(>uc))
+    return;
+
+    if (GUC_FIRMWARE_VER(guc) < MAKE_GUC_VER(70, 10, 0))
+    return;

should this be <= ?
No. GuC 70.10.0 is when w/a KLVs were introduced. So we want to skip on 
any version that is prior to 70.10.0.



+
+    GEM_BUG_ON(iosys_map_is_null(>ads_map));
+    offset = guc_ads_waklv_offset(guc);
+    remain = guc_ads_waklv_size(guc);
+
+    /* Add workarounds here */
+

extra blank line?
The

Re: [Intel-gfx] [PATCH 3/4] drm/i915/guc: Add support for w/a KLVs

2023-10-06 Thread Belgaumkar, Vinay 



On 9/15/2023 2:55 PM, john.c.harri...@intel.com wrote:

From: John Harrison 

To prevent running out of bits, new w/a enable flags are being added
via a KLV system instead of a 32 bit flags word.

Signed-off-by: John Harrison 
---
  .../gpu/drm/i915/gt/uc/abi/guc_errors_abi.h   |  1 +
  drivers/gpu/drm/i915/gt/uc/intel_guc.h|  3 +
  drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c| 64 ++-
  drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c |  6 ++
  drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h   |  5 +-
  5 files changed, 77 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_errors_abi.h 
b/drivers/gpu/drm/i915/gt/uc/abi/guc_errors_abi.h
index dabeaf4f245f3..00d6402333f8e 100644
--- a/drivers/gpu/drm/i915/gt/uc/abi/guc_errors_abi.h
+++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_errors_abi.h
@@ -36,6 +36,7 @@ enum intel_guc_load_status {
INTEL_GUC_LOAD_STATUS_INVALID_INIT_DATA_RANGE_START,
INTEL_GUC_LOAD_STATUS_MPU_DATA_INVALID = 0x73,
INTEL_GUC_LOAD_STATUS_INIT_MMIO_SAVE_RESTORE_INVALID   = 0x74,
+   INTEL_GUC_LOAD_STATUS_KLV_WORKAROUND_INIT_ERROR= 0x75,
INTEL_GUC_LOAD_STATUS_INVALID_INIT_DATA_RANGE_END,
  
  	INTEL_GUC_LOAD_STATUS_READY= 0xF0,

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h 
b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
index 6c392bad29c19..3b1fc5f96306b 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
@@ -186,6 +186,8 @@ struct intel_guc {
struct guc_mmio_reg *ads_regset;
/** @ads_golden_ctxt_size: size of the golden contexts in the ADS */
u32 ads_golden_ctxt_size;
+   /** @ads_waklv_size: size of workaround KLVs */
+   u32 ads_waklv_size;
/** @ads_capture_size: size of register lists in the ADS used for error 
capture */
u32 ads_capture_size;
/** @ads_engine_usage_size: size of engine usage in the ADS */
@@ -295,6 +297,7 @@ struct intel_guc {
  #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)
+#define GUC_FIRMWARE_VER(guc)  
MAKE_GUC_VER_STRUCT((guc)->fw.file_selected.ver)
  
  static inline struct intel_guc *log_to_guc(struct intel_guc_log *log)

  {
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c 
b/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c
index 63724e17829a7..792910af3a481 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c
@@ -46,6 +46,10 @@
   *  +---+
   *  | padding   |
   *  +---+ <== 4K aligned
+ *  | w/a KLVs  |
+ *  +---+
+ *  | padding   |
+ *  +---+ <== 4K aligned
   *  | capture lists |
   *  +---+
   *  | padding   |
@@ -88,6 +92,11 @@ static u32 guc_ads_golden_ctxt_size(struct intel_guc *guc)
return PAGE_ALIGN(guc->ads_golden_ctxt_size);
  }
  
+static u32 guc_ads_waklv_size(struct intel_guc *guc)

+{
+   return PAGE_ALIGN(guc->ads_waklv_size);
+}
+
  static u32 guc_ads_capture_size(struct intel_guc *guc)
  {
return PAGE_ALIGN(guc->ads_capture_size);
@@ -113,7 +122,7 @@ static u32 guc_ads_golden_ctxt_offset(struct intel_guc *guc)
return PAGE_ALIGN(offset);
  }
  
-static u32 guc_ads_capture_offset(struct intel_guc *guc)

+static u32 guc_ads_waklv_offset(struct intel_guc *guc)
  {
u32 offset;
  
@@ -123,6 +132,16 @@ static u32 guc_ads_capture_offset(struct intel_guc *guc)

return PAGE_ALIGN(offset);
  }
  
+static u32 guc_ads_capture_offset(struct intel_guc *guc)

+{
+   u32 offset;
+
+   offset = guc_ads_waklv_offset(guc) +
+guc_ads_waklv_size(guc);
+
+   return PAGE_ALIGN(offset);
+}
+
  static u32 guc_ads_private_data_offset(struct intel_guc *guc)
  {
u32 offset;
@@ -791,6 +810,40 @@ guc_capture_prep_lists(struct intel_guc *guc)
return PAGE_ALIGN(total_size);
  }
  
+static void guc_waklv_init(struct intel_guc *guc)

+{
+   struct intel_gt *gt = guc_to_gt(guc);
+   u32 offset, addr_ggtt, remain, size;
+
+   if (!intel_uc_uses_guc_submission(>uc))
+   return;
+
+   if (GUC_FIRMWARE_VER(guc) < MAKE_GUC_VER(70, 10, 0))
+   return;

should this be <= ?

+
+   GEM_BUG_ON(iosys_map_is_null(>ads_map));
+   offset = guc_ads_waklv_offset(guc);
+   remain = guc_ads_waklv_size(guc);
+
+   /* Add workarounds here */
+

extra blank line?

+   size = guc_ads_waklv_size(guc) - remain;