All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5 0/4] GuC HWCONFIG with documentation
@ 2022-02-22 10:36 ` Jordan Justen
  0 siblings, 0 replies; 32+ messages in thread
From: Jordan Justen @ 2022-02-22 10:36 UTC (permalink / raw)
  To: intel-gfx; +Cc: Jordan Justen, dri-devel

This is John/Rodrigo's 2 patches with some minor changes, and I added
2 patches.

"drm/i915/uapi: Add query for hwconfig blob" was changed:

 * Rename DRM_I915_QUERY_HWCONFIG_TABLE to DRM_I915_QUERY_HWCONFIG_BLOB
   as requested by Joonas.

 * Reword commit message

 * I added Acked-by to this patch, but this only applies in the
   context of this version of the patchset. If my changes are
   rejected, then please *do not* add my Acked-by to the other series.

   In particular, I do not want my Acked-by on the patch if the patch
   mentions the HWCONFIG format, but is not willing to add that to the
   actual uAPI.

   I also do not want my Acked-by on it if it mentions "consolidation"
   of this data. Since we are dealing with open source projects (aside
   from GuC), this doesn't seem appropriate.

"drm/i915/uapi: Add struct drm_i915_query_hwconfig_blob_item" adds a
struct to the uAPI and documents the return value for
DRM_I915_QUERY_HWCONFIG_BLOB. (Except, keys / values are still
deferred to the PRM.)

"drm/i915/guc: Verify hwconfig blob matches supported format" does the
simple verification of the blob to make sure it matches what the uAPI
documents.

v2:
 * Fix -Werror errors.
 * Rebase to drm-intel/for-linux-next instead of
   drm-intel/for-linux-next-gt, as this seems to be what CI wants.
 * Fix u32 -> __u32.
 * Add commit message for "Verify hwconfig blob" patch as requested by
   Tvrtko.
 * Reword text added to i915_drm.h as requested by Tvrtko. (Attempting
   to indicate the overall blob ends right at the last blob item.)

v3:
 * Add several changes suggested by Tvrtko in the "Verify hwconfig
   blob", along with some tweaks to i915_drm.h from the feedback for
   the same patch.

v4:
 * Rewrite verify_hwconfig_blob() to hopefully be clearer without
   relying on comments so much, and add various suggestions from
   Michal.
 * Michal also had some suggestions in John's "drm/i915/guc: Add fetch
   of hwconfig table" patch. I held off on making any of these changes
   in this version.

v5:
 * Add many changes suggested by Michal in John's "drm/i915/guc: Add
   fetch of hwconfig table" patch.
 * Fix documenation formatting as suggested by Daniel in
   "drm/i915/uapi: Add struct drm_i915_query_hwconfig_blob_item"

John Harrison (1):
  drm/i915/guc: Add fetch of hwconfig table

Jordan Justen (2):
  drm/i915/uapi: Add struct drm_i915_query_hwconfig_blob_item
  drm/i915/guc: Verify hwconfig blob matches supported format

Rodrigo Vivi (1):
  drm/i915/uapi: Add query for hwconfig blob

 drivers/gpu/drm/i915/Makefile                 |   1 +
 .../gpu/drm/i915/gt/uc/abi/guc_actions_abi.h  |   1 +
 .../gpu/drm/i915/gt/uc/abi/guc_errors_abi.h   |   4 +
 drivers/gpu/drm/i915/gt/uc/intel_guc.h        |   3 +
 .../gpu/drm/i915/gt/uc/intel_guc_hwconfig.c   | 187 ++++++++++++++++++
 .../gpu/drm/i915/gt/uc/intel_guc_hwconfig.h   |  19 ++
 drivers/gpu/drm/i915/gt/uc/intel_uc.c         |   7 +
 drivers/gpu/drm/i915/i915_pci.c               |   1 +
 drivers/gpu/drm/i915/i915_query.c             |  23 +++
 drivers/gpu/drm/i915/intel_device_info.h      |   1 +
 include/uapi/drm/i915_drm.h                   |  44 +++++
 11 files changed, 291 insertions(+)
 create mode 100644 drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c
 create mode 100644 drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.h

-- 
2.34.1


^ permalink raw reply	[flat|nested] 32+ messages in thread
* [Intel-gfx] [PATCH v5 0/4] GuC HWCONFIG with documentation
@ 2022-02-22 10:36 ` Jordan Justen
  0 siblings, 0 replies; 32+ messages in thread
From: Jordan Justen @ 2022-02-22 10:36 UTC (permalink / raw)
  To: intel-gfx; +Cc: dri-devel

This is John/Rodrigo's 2 patches with some minor changes, and I added
2 patches.

"drm/i915/uapi: Add query for hwconfig blob" was changed:

 * Rename DRM_I915_QUERY_HWCONFIG_TABLE to DRM_I915_QUERY_HWCONFIG_BLOB
   as requested by Joonas.

 * Reword commit message

 * I added Acked-by to this patch, but this only applies in the
   context of this version of the patchset. If my changes are
   rejected, then please *do not* add my Acked-by to the other series.

   In particular, I do not want my Acked-by on the patch if the patch
   mentions the HWCONFIG format, but is not willing to add that to the
   actual uAPI.

   I also do not want my Acked-by on it if it mentions "consolidation"
   of this data. Since we are dealing with open source projects (aside
   from GuC), this doesn't seem appropriate.

"drm/i915/uapi: Add struct drm_i915_query_hwconfig_blob_item" adds a
struct to the uAPI and documents the return value for
DRM_I915_QUERY_HWCONFIG_BLOB. (Except, keys / values are still
deferred to the PRM.)

"drm/i915/guc: Verify hwconfig blob matches supported format" does the
simple verification of the blob to make sure it matches what the uAPI
documents.

v2:
 * Fix -Werror errors.
 * Rebase to drm-intel/for-linux-next instead of
   drm-intel/for-linux-next-gt, as this seems to be what CI wants.
 * Fix u32 -> __u32.
 * Add commit message for "Verify hwconfig blob" patch as requested by
   Tvrtko.
 * Reword text added to i915_drm.h as requested by Tvrtko. (Attempting
   to indicate the overall blob ends right at the last blob item.)

v3:
 * Add several changes suggested by Tvrtko in the "Verify hwconfig
   blob", along with some tweaks to i915_drm.h from the feedback for
   the same patch.

v4:
 * Rewrite verify_hwconfig_blob() to hopefully be clearer without
   relying on comments so much, and add various suggestions from
   Michal.
 * Michal also had some suggestions in John's "drm/i915/guc: Add fetch
   of hwconfig table" patch. I held off on making any of these changes
   in this version.

v5:
 * Add many changes suggested by Michal in John's "drm/i915/guc: Add
   fetch of hwconfig table" patch.
 * Fix documenation formatting as suggested by Daniel in
   "drm/i915/uapi: Add struct drm_i915_query_hwconfig_blob_item"

John Harrison (1):
  drm/i915/guc: Add fetch of hwconfig table

Jordan Justen (2):
  drm/i915/uapi: Add struct drm_i915_query_hwconfig_blob_item
  drm/i915/guc: Verify hwconfig blob matches supported format

Rodrigo Vivi (1):
  drm/i915/uapi: Add query for hwconfig blob

 drivers/gpu/drm/i915/Makefile                 |   1 +
 .../gpu/drm/i915/gt/uc/abi/guc_actions_abi.h  |   1 +
 .../gpu/drm/i915/gt/uc/abi/guc_errors_abi.h   |   4 +
 drivers/gpu/drm/i915/gt/uc/intel_guc.h        |   3 +
 .../gpu/drm/i915/gt/uc/intel_guc_hwconfig.c   | 187 ++++++++++++++++++
 .../gpu/drm/i915/gt/uc/intel_guc_hwconfig.h   |  19 ++
 drivers/gpu/drm/i915/gt/uc/intel_uc.c         |   7 +
 drivers/gpu/drm/i915/i915_pci.c               |   1 +
 drivers/gpu/drm/i915/i915_query.c             |  23 +++
 drivers/gpu/drm/i915/intel_device_info.h      |   1 +
 include/uapi/drm/i915_drm.h                   |  44 +++++
 11 files changed, 291 insertions(+)
 create mode 100644 drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c
 create mode 100644 drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.h

-- 
2.34.1


^ permalink raw reply	[flat|nested] 32+ messages in thread
* [PATCH v5 1/4] drm/i915/guc: Add fetch of hwconfig table
  2022-02-22 10:36 ` [Intel-gfx] " Jordan Justen
@ 2022-02-22 10:36   ` Jordan Justen
  -1 siblings, 0 replies; 32+ messages in thread
From: Jordan Justen @ 2022-02-22 10:36 UTC (permalink / raw)
  To: intel-gfx
  Cc: Matthew Brost, Jordan Justen, dri-devel, Jon Bloomfield,
	Rodrigo Vivi, John Harrison, Michal Wajdeczko

From: John Harrison <John.C.Harrison@Intel.com>

Implement support for fetching the hardware description table from the
GuC. The call is made twice - once without a destination buffer to
query the size and then a second time to fill in the buffer.

Note that the table is only available on ADL-P and later platforms.

v5 (of Jordan's posting):
 * Various changes made by Jordan and recommended by Michal
   - Makefile ordering
   - Adjust "struct intel_guc_hwconfig hwconfig" comment
   - Set Copyright year to 2022 in intel_guc_hwconfig.c/.h
   - Drop inline from hwconfig_to_guc()
   - Replace hwconfig param with guc in __guc_action_get_hwconfig()
   - Move zero size check into guc_hwconfig_discover_size()
   - Change comment to say zero size offset/size is needed to get size
   - Add has_guc_hwconfig to devinfo and drop has_table()
   - Change drm_err to notice in __uc_init_hw() and use %pe

Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
Reviewed-by: Matthew Brost <matthew.brost@intel.com>
Acked-by: Jon Bloomfield <jon.bloomfield@intel.com>
Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
---
 drivers/gpu/drm/i915/Makefile                 |   1 +
 .../gpu/drm/i915/gt/uc/abi/guc_actions_abi.h  |   1 +
 .../gpu/drm/i915/gt/uc/abi/guc_errors_abi.h   |   4 +
 drivers/gpu/drm/i915/gt/uc/intel_guc.h        |   3 +
 .../gpu/drm/i915/gt/uc/intel_guc_hwconfig.c   | 145 ++++++++++++++++++
 .../gpu/drm/i915/gt/uc/intel_guc_hwconfig.h   |  19 +++
 drivers/gpu/drm/i915/gt/uc/intel_uc.c         |   7 +
 drivers/gpu/drm/i915/i915_pci.c               |   1 +
 drivers/gpu/drm/i915/intel_device_info.h      |   1 +
 9 files changed, 182 insertions(+)
 create mode 100644 drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c
 create mode 100644 drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.h

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index e9ce09620eb5..661f1afb51d7 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -188,6 +188,7 @@ i915-y += gt/uc/intel_uc.o \
 	  gt/uc/intel_guc_ct.o \
 	  gt/uc/intel_guc_debugfs.o \
 	  gt/uc/intel_guc_fw.o \
+	  gt/uc/intel_guc_hwconfig.o \
 	  gt/uc/intel_guc_log.o \
 	  gt/uc/intel_guc_log_debugfs.o \
 	  gt/uc/intel_guc_rc.o \
diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h
index fe5d7d261797..4a61c819f32b 100644
--- a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h
+++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h
@@ -137,6 +137,7 @@ enum intel_guc_action {
 	INTEL_GUC_ACTION_ENGINE_FAILURE_NOTIFICATION = 0x1009,
 	INTEL_GUC_ACTION_SETUP_PC_GUCRC = 0x3004,
 	INTEL_GUC_ACTION_AUTHENTICATE_HUC = 0x4000,
+	INTEL_GUC_ACTION_GET_HWCONFIG = 0x4100,
 	INTEL_GUC_ACTION_REGISTER_CONTEXT = 0x4502,
 	INTEL_GUC_ACTION_DEREGISTER_CONTEXT = 0x4503,
 	INTEL_GUC_ACTION_REGISTER_COMMAND_TRANSPORT_BUFFER = 0x4505,
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 488b6061ee89..f9e2a6aaef4a 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
@@ -8,6 +8,10 @@
 
 enum intel_guc_response_status {
 	INTEL_GUC_RESPONSE_STATUS_SUCCESS = 0x0,
+	INTEL_GUC_RESPONSE_NOT_SUPPORTED = 0x20,
+	INTEL_GUC_RESPONSE_NO_ATTRIBUTE_TABLE = 0x201,
+	INTEL_GUC_RESPONSE_NO_DECRYPTION_KEY = 0x202,
+	INTEL_GUC_RESPONSE_DECRYPTION_FAILED = 0x204,
 	INTEL_GUC_RESPONSE_STATUS_GENERIC_FAIL = 0xF000,
 };
 
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
index f9240d4baa69..2058eb8c3d0c 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
@@ -13,6 +13,7 @@
 #include "intel_guc_fw.h"
 #include "intel_guc_fwif.h"
 #include "intel_guc_ct.h"
+#include "intel_guc_hwconfig.h"
 #include "intel_guc_log.h"
 #include "intel_guc_reg.h"
 #include "intel_guc_slpc_types.h"
@@ -37,6 +38,8 @@ struct intel_guc {
 	struct intel_guc_ct ct;
 	/** @slpc: sub-structure containing SLPC related data and objects */
 	struct intel_guc_slpc slpc;
+	/** @hwconfig: data related to hardware configuration KLV blob */
+	struct intel_guc_hwconfig hwconfig;
 
 	/** @sched_engine: Global engine used to submit requests to GuC */
 	struct i915_sched_engine *sched_engine;
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c
new file mode 100644
index 000000000000..ad289603460c
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c
@@ -0,0 +1,145 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2022 Intel Corporation
+ */
+
+#include "gt/intel_gt.h"
+#include "i915_drv.h"
+#include "i915_memcpy.h"
+#include "intel_guc_hwconfig.h"
+
+static struct intel_guc *hwconfig_to_guc(struct intel_guc_hwconfig *hwconfig)
+{
+	return container_of(hwconfig, struct intel_guc, hwconfig);
+}
+
+/*
+ * GuC has a blob containing hardware configuration information (HWConfig).
+ * This is formatted as a simple and flexible KLV (Key/Length/Value) table.
+ *
+ * For example, a minimal version could be:
+ *   enum device_attr {
+ *     ATTR_SOME_VALUE = 0,
+ *     ATTR_SOME_MASK  = 1,
+ *   };
+ *
+ *   static const u32 hwconfig[] = {
+ *     ATTR_SOME_VALUE,
+ *     1,		// Value Length in DWords
+ *     8,		// Value
+ *
+ *     ATTR_SOME_MASK,
+ *     3,
+ *     0x00FFFFFFFF, 0xFFFFFFFF, 0xFF000000,
+ *   };
+ *
+ * The attribute ids are defined in a hardware spec.
+ */
+
+static int __guc_action_get_hwconfig(struct intel_guc *guc,
+				     u32 ggtt_offset, u32 ggtt_size)
+{
+	u32 action[] = {
+		INTEL_GUC_ACTION_GET_HWCONFIG,
+		ggtt_offset,
+		0, /* upper 32 bits of address */
+		ggtt_size,
+	};
+	int ret;
+
+	ret = intel_guc_send_mmio(guc, action, ARRAY_SIZE(action), NULL, 0);
+	if (ret == -ENXIO)
+		return -ENOENT;
+
+	return ret;
+}
+
+static int guc_hwconfig_discover_size(struct intel_guc_hwconfig *hwconfig)
+{
+	struct intel_guc *guc = hwconfig_to_guc(hwconfig);
+	int ret;
+
+	/* Sending a query with zero offset and size will return the
+	 * size of the blob.
+	 */
+	ret = __guc_action_get_hwconfig(guc, 0, 0);
+	if (ret < 0)
+		return ret;
+
+	if (ret == 0)
+		return -EINVAL;
+
+	hwconfig->size = ret;
+	return 0;
+}
+
+static int guc_hwconfig_fill_buffer(struct intel_guc_hwconfig *hwconfig)
+{
+	struct intel_guc *guc = hwconfig_to_guc(hwconfig);
+	struct i915_vma *vma;
+	u32 ggtt_offset;
+	void *vaddr;
+	int ret;
+
+	GEM_BUG_ON(!hwconfig->size);
+
+	ret = intel_guc_allocate_and_map_vma(guc, hwconfig->size, &vma, &vaddr);
+	if (ret)
+		return ret;
+
+	ggtt_offset = intel_guc_ggtt_offset(guc, vma);
+
+	ret = __guc_action_get_hwconfig(guc, ggtt_offset, hwconfig->size);
+	if (ret >= 0)
+		memcpy(hwconfig->ptr, vaddr, hwconfig->size);
+
+	i915_vma_unpin_and_release(&vma, I915_VMA_RELEASE_MAP);
+
+	return ret;
+}
+
+/**
+ * intel_guc_hwconfig_fini - Finalize the HWConfig
+ *
+ * Free up the memory allocation holding the table.
+ */
+void intel_guc_hwconfig_fini(struct intel_guc_hwconfig *hwconfig)
+{
+	kfree(hwconfig->ptr);
+	hwconfig->size = 0;
+	hwconfig->ptr = NULL;
+}
+
+/**
+ * intel_guc_hwconfig_init - Initialize the HWConfig
+ *
+ * Retrieve the HWConfig table from the GuC and save it away in a local memory
+ * allocation. It can then be queried on demand by other users later on.
+ */
+int intel_guc_hwconfig_init(struct intel_guc_hwconfig *hwconfig)
+{
+	struct intel_guc *guc = hwconfig_to_guc(hwconfig);
+	struct drm_i915_private *i915 = guc_to_gt(guc)->i915;
+	int ret;
+
+	if (!INTEL_INFO(i915)->has_guc_hwconfig)
+		return 0;
+
+	ret = guc_hwconfig_discover_size(hwconfig);
+	if (ret)
+		return ret;
+
+	hwconfig->ptr = kmalloc(hwconfig->size, GFP_KERNEL);
+	if (!hwconfig->ptr) {
+		hwconfig->size = 0;
+		return -ENOMEM;
+	}
+
+	ret = guc_hwconfig_fill_buffer(hwconfig);
+	if (ret < 0) {
+		intel_guc_hwconfig_fini(hwconfig);
+		return ret;
+	}
+
+	return 0;
+}
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.h
new file mode 100644
index 000000000000..bfb90ae168dc
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2022 Intel Corporation
+ */
+
+#ifndef _INTEL_GUC_HWCONFIG_H_
+#define _INTEL_GUC_HWCONFIG_H_
+
+#include <linux/types.h>
+
+struct intel_guc_hwconfig {
+	u32 size;
+	void *ptr;
+};
+
+int intel_guc_hwconfig_init(struct intel_guc_hwconfig *hwconfig);
+void intel_guc_hwconfig_fini(struct intel_guc_hwconfig *hwconfig);
+
+#endif /* _INTEL_GUC_HWCONFIG_H_ */
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
index 09ed29df67bc..0cefa2a95190 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
@@ -489,6 +489,11 @@ static int __uc_init_hw(struct intel_uc *uc)
 	if (ret)
 		goto err_log_capture;
 
+	ret = intel_guc_hwconfig_init(&guc->hwconfig);
+	if (ret)
+		drm_notice(&i915->drm, "Failed to retrieve hwconfig table: %pe\n",
+			   ERR_PTR(ret));
+
 	ret = guc_enable_communication(guc);
 	if (ret)
 		goto err_log_capture;
@@ -562,6 +567,8 @@ static void __uc_fini_hw(struct intel_uc *uc)
 	if (intel_uc_uses_guc_submission(uc))
 		intel_guc_submission_disable(guc);
 
+	intel_guc_hwconfig_fini(&guc->hwconfig);
+
 	__uc_sanitize(uc);
 }
 
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index 76e590fcb903..1d31e35a5154 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -990,6 +990,7 @@ static const struct intel_device_info adl_p_info = {
 		BIT(RCS0) | BIT(BCS0) | BIT(VECS0) | BIT(VCS0) | BIT(VCS2),
 	.ppgtt_size = 48,
 	.dma_mask_size = 39,
+	.has_guc_hwconfig = 1,
 };
 
 #undef GEN
diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
index 3699b1c539ea..82d8d6bc30ff 100644
--- a/drivers/gpu/drm/i915/intel_device_info.h
+++ b/drivers/gpu/drm/i915/intel_device_info.h
@@ -133,6 +133,7 @@ enum intel_ppgtt_type {
 	func(gpu_reset_clobbers_display); \
 	func(has_reset_engine); \
 	func(has_global_mocs); \
+	func(has_guc_hwconfig); \
 	func(has_gt_uc); \
 	func(has_l3_dpf); \
 	func(has_llc); \
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread
* [Intel-gfx] [PATCH v5 1/4] drm/i915/guc: Add fetch of hwconfig table
@ 2022-02-22 10:36   ` Jordan Justen
  0 siblings, 0 replies; 32+ messages in thread
From: Jordan Justen @ 2022-02-22 10:36 UTC (permalink / raw)
  To: intel-gfx; +Cc: dri-devel, Rodrigo Vivi

From: John Harrison <John.C.Harrison@Intel.com>

Implement support for fetching the hardware description table from the
GuC. The call is made twice - once without a destination buffer to
query the size and then a second time to fill in the buffer.

Note that the table is only available on ADL-P and later platforms.

v5 (of Jordan's posting):
 * Various changes made by Jordan and recommended by Michal
   - Makefile ordering
   - Adjust "struct intel_guc_hwconfig hwconfig" comment
   - Set Copyright year to 2022 in intel_guc_hwconfig.c/.h
   - Drop inline from hwconfig_to_guc()
   - Replace hwconfig param with guc in __guc_action_get_hwconfig()
   - Move zero size check into guc_hwconfig_discover_size()
   - Change comment to say zero size offset/size is needed to get size
   - Add has_guc_hwconfig to devinfo and drop has_table()
   - Change drm_err to notice in __uc_init_hw() and use %pe

Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
Reviewed-by: Matthew Brost <matthew.brost@intel.com>
Acked-by: Jon Bloomfield <jon.bloomfield@intel.com>
Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
---
 drivers/gpu/drm/i915/Makefile                 |   1 +
 .../gpu/drm/i915/gt/uc/abi/guc_actions_abi.h  |   1 +
 .../gpu/drm/i915/gt/uc/abi/guc_errors_abi.h   |   4 +
 drivers/gpu/drm/i915/gt/uc/intel_guc.h        |   3 +
 .../gpu/drm/i915/gt/uc/intel_guc_hwconfig.c   | 145 ++++++++++++++++++
 .../gpu/drm/i915/gt/uc/intel_guc_hwconfig.h   |  19 +++
 drivers/gpu/drm/i915/gt/uc/intel_uc.c         |   7 +
 drivers/gpu/drm/i915/i915_pci.c               |   1 +
 drivers/gpu/drm/i915/intel_device_info.h      |   1 +
 9 files changed, 182 insertions(+)
 create mode 100644 drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c
 create mode 100644 drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.h

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index e9ce09620eb5..661f1afb51d7 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -188,6 +188,7 @@ i915-y += gt/uc/intel_uc.o \
 	  gt/uc/intel_guc_ct.o \
 	  gt/uc/intel_guc_debugfs.o \
 	  gt/uc/intel_guc_fw.o \
+	  gt/uc/intel_guc_hwconfig.o \
 	  gt/uc/intel_guc_log.o \
 	  gt/uc/intel_guc_log_debugfs.o \
 	  gt/uc/intel_guc_rc.o \
diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h
index fe5d7d261797..4a61c819f32b 100644
--- a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h
+++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h
@@ -137,6 +137,7 @@ enum intel_guc_action {
 	INTEL_GUC_ACTION_ENGINE_FAILURE_NOTIFICATION = 0x1009,
 	INTEL_GUC_ACTION_SETUP_PC_GUCRC = 0x3004,
 	INTEL_GUC_ACTION_AUTHENTICATE_HUC = 0x4000,
+	INTEL_GUC_ACTION_GET_HWCONFIG = 0x4100,
 	INTEL_GUC_ACTION_REGISTER_CONTEXT = 0x4502,
 	INTEL_GUC_ACTION_DEREGISTER_CONTEXT = 0x4503,
 	INTEL_GUC_ACTION_REGISTER_COMMAND_TRANSPORT_BUFFER = 0x4505,
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 488b6061ee89..f9e2a6aaef4a 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
@@ -8,6 +8,10 @@
 
 enum intel_guc_response_status {
 	INTEL_GUC_RESPONSE_STATUS_SUCCESS = 0x0,
+	INTEL_GUC_RESPONSE_NOT_SUPPORTED = 0x20,
+	INTEL_GUC_RESPONSE_NO_ATTRIBUTE_TABLE = 0x201,
+	INTEL_GUC_RESPONSE_NO_DECRYPTION_KEY = 0x202,
+	INTEL_GUC_RESPONSE_DECRYPTION_FAILED = 0x204,
 	INTEL_GUC_RESPONSE_STATUS_GENERIC_FAIL = 0xF000,
 };
 
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
index f9240d4baa69..2058eb8c3d0c 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
@@ -13,6 +13,7 @@
 #include "intel_guc_fw.h"
 #include "intel_guc_fwif.h"
 #include "intel_guc_ct.h"
+#include "intel_guc_hwconfig.h"
 #include "intel_guc_log.h"
 #include "intel_guc_reg.h"
 #include "intel_guc_slpc_types.h"
@@ -37,6 +38,8 @@ struct intel_guc {
 	struct intel_guc_ct ct;
 	/** @slpc: sub-structure containing SLPC related data and objects */
 	struct intel_guc_slpc slpc;
+	/** @hwconfig: data related to hardware configuration KLV blob */
+	struct intel_guc_hwconfig hwconfig;
 
 	/** @sched_engine: Global engine used to submit requests to GuC */
 	struct i915_sched_engine *sched_engine;
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c
new file mode 100644
index 000000000000..ad289603460c
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c
@@ -0,0 +1,145 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2022 Intel Corporation
+ */
+
+#include "gt/intel_gt.h"
+#include "i915_drv.h"
+#include "i915_memcpy.h"
+#include "intel_guc_hwconfig.h"
+
+static struct intel_guc *hwconfig_to_guc(struct intel_guc_hwconfig *hwconfig)
+{
+	return container_of(hwconfig, struct intel_guc, hwconfig);
+}
+
+/*
+ * GuC has a blob containing hardware configuration information (HWConfig).
+ * This is formatted as a simple and flexible KLV (Key/Length/Value) table.
+ *
+ * For example, a minimal version could be:
+ *   enum device_attr {
+ *     ATTR_SOME_VALUE = 0,
+ *     ATTR_SOME_MASK  = 1,
+ *   };
+ *
+ *   static const u32 hwconfig[] = {
+ *     ATTR_SOME_VALUE,
+ *     1,		// Value Length in DWords
+ *     8,		// Value
+ *
+ *     ATTR_SOME_MASK,
+ *     3,
+ *     0x00FFFFFFFF, 0xFFFFFFFF, 0xFF000000,
+ *   };
+ *
+ * The attribute ids are defined in a hardware spec.
+ */
+
+static int __guc_action_get_hwconfig(struct intel_guc *guc,
+				     u32 ggtt_offset, u32 ggtt_size)
+{
+	u32 action[] = {
+		INTEL_GUC_ACTION_GET_HWCONFIG,
+		ggtt_offset,
+		0, /* upper 32 bits of address */
+		ggtt_size,
+	};
+	int ret;
+
+	ret = intel_guc_send_mmio(guc, action, ARRAY_SIZE(action), NULL, 0);
+	if (ret == -ENXIO)
+		return -ENOENT;
+
+	return ret;
+}
+
+static int guc_hwconfig_discover_size(struct intel_guc_hwconfig *hwconfig)
+{
+	struct intel_guc *guc = hwconfig_to_guc(hwconfig);
+	int ret;
+
+	/* Sending a query with zero offset and size will return the
+	 * size of the blob.
+	 */
+	ret = __guc_action_get_hwconfig(guc, 0, 0);
+	if (ret < 0)
+		return ret;
+
+	if (ret == 0)
+		return -EINVAL;
+
+	hwconfig->size = ret;
+	return 0;
+}
+
+static int guc_hwconfig_fill_buffer(struct intel_guc_hwconfig *hwconfig)
+{
+	struct intel_guc *guc = hwconfig_to_guc(hwconfig);
+	struct i915_vma *vma;
+	u32 ggtt_offset;
+	void *vaddr;
+	int ret;
+
+	GEM_BUG_ON(!hwconfig->size);
+
+	ret = intel_guc_allocate_and_map_vma(guc, hwconfig->size, &vma, &vaddr);
+	if (ret)
+		return ret;
+
+	ggtt_offset = intel_guc_ggtt_offset(guc, vma);
+
+	ret = __guc_action_get_hwconfig(guc, ggtt_offset, hwconfig->size);
+	if (ret >= 0)
+		memcpy(hwconfig->ptr, vaddr, hwconfig->size);
+
+	i915_vma_unpin_and_release(&vma, I915_VMA_RELEASE_MAP);
+
+	return ret;
+}
+
+/**
+ * intel_guc_hwconfig_fini - Finalize the HWConfig
+ *
+ * Free up the memory allocation holding the table.
+ */
+void intel_guc_hwconfig_fini(struct intel_guc_hwconfig *hwconfig)
+{
+	kfree(hwconfig->ptr);
+	hwconfig->size = 0;
+	hwconfig->ptr = NULL;
+}
+
+/**
+ * intel_guc_hwconfig_init - Initialize the HWConfig
+ *
+ * Retrieve the HWConfig table from the GuC and save it away in a local memory
+ * allocation. It can then be queried on demand by other users later on.
+ */
+int intel_guc_hwconfig_init(struct intel_guc_hwconfig *hwconfig)
+{
+	struct intel_guc *guc = hwconfig_to_guc(hwconfig);
+	struct drm_i915_private *i915 = guc_to_gt(guc)->i915;
+	int ret;
+
+	if (!INTEL_INFO(i915)->has_guc_hwconfig)
+		return 0;
+
+	ret = guc_hwconfig_discover_size(hwconfig);
+	if (ret)
+		return ret;
+
+	hwconfig->ptr = kmalloc(hwconfig->size, GFP_KERNEL);
+	if (!hwconfig->ptr) {
+		hwconfig->size = 0;
+		return -ENOMEM;
+	}
+
+	ret = guc_hwconfig_fill_buffer(hwconfig);
+	if (ret < 0) {
+		intel_guc_hwconfig_fini(hwconfig);
+		return ret;
+	}
+
+	return 0;
+}
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.h
new file mode 100644
index 000000000000..bfb90ae168dc
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2022 Intel Corporation
+ */
+
+#ifndef _INTEL_GUC_HWCONFIG_H_
+#define _INTEL_GUC_HWCONFIG_H_
+
+#include <linux/types.h>
+
+struct intel_guc_hwconfig {
+	u32 size;
+	void *ptr;
+};
+
+int intel_guc_hwconfig_init(struct intel_guc_hwconfig *hwconfig);
+void intel_guc_hwconfig_fini(struct intel_guc_hwconfig *hwconfig);
+
+#endif /* _INTEL_GUC_HWCONFIG_H_ */
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
index 09ed29df67bc..0cefa2a95190 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
@@ -489,6 +489,11 @@ static int __uc_init_hw(struct intel_uc *uc)
 	if (ret)
 		goto err_log_capture;
 
+	ret = intel_guc_hwconfig_init(&guc->hwconfig);
+	if (ret)
+		drm_notice(&i915->drm, "Failed to retrieve hwconfig table: %pe\n",
+			   ERR_PTR(ret));
+
 	ret = guc_enable_communication(guc);
 	if (ret)
 		goto err_log_capture;
@@ -562,6 +567,8 @@ static void __uc_fini_hw(struct intel_uc *uc)
 	if (intel_uc_uses_guc_submission(uc))
 		intel_guc_submission_disable(guc);
 
+	intel_guc_hwconfig_fini(&guc->hwconfig);
+
 	__uc_sanitize(uc);
 }
 
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index 76e590fcb903..1d31e35a5154 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -990,6 +990,7 @@ static const struct intel_device_info adl_p_info = {
 		BIT(RCS0) | BIT(BCS0) | BIT(VECS0) | BIT(VCS0) | BIT(VCS2),
 	.ppgtt_size = 48,
 	.dma_mask_size = 39,
+	.has_guc_hwconfig = 1,
 };
 
 #undef GEN
diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
index 3699b1c539ea..82d8d6bc30ff 100644
--- a/drivers/gpu/drm/i915/intel_device_info.h
+++ b/drivers/gpu/drm/i915/intel_device_info.h
@@ -133,6 +133,7 @@ enum intel_ppgtt_type {
 	func(gpu_reset_clobbers_display); \
 	func(has_reset_engine); \
 	func(has_global_mocs); \
+	func(has_guc_hwconfig); \
 	func(has_gt_uc); \
 	func(has_l3_dpf); \
 	func(has_llc); \
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread
* [PATCH v5 2/4] drm/i915/uapi: Add query for hwconfig blob
  2022-02-22 10:36 ` [Intel-gfx] " Jordan Justen
@ 2022-02-22 10:36   ` Jordan Justen
  -1 siblings, 0 replies; 32+ messages in thread
From: Jordan Justen @ 2022-02-22 10:36 UTC (permalink / raw)
  To: intel-gfx
  Cc: Matthew Brost, Tvrtko Ursulin, Kenneth Graunke, Jordan Justen,
	dri-devel, Slawomir Milczarek, Jon Bloomfield, Rodrigo Vivi,
	John Harrison, Michal Wajdeczko

From: Rodrigo Vivi <rodrigo.vivi@intel.com>

The DRM_I915_QUERY_HWCONFIG_BLOB query item returns a blob of data
which it receives from the GuC software. This blob provides some
useful data about the hardware for drivers.

Although the blob is not fully documented at this time, the basic
format is an array of u32 values. The array is a simple and flexible
KLV (Key/Length/Value) formatted table. For example, it could be just:
enum device_attr { ATTR_SOME_VALUE = 0, ATTR_SOME_MASK = 1, };

  static const u32 hwconfig[] = {
      ATTR_SOME_VALUE,
      1,             // Value Length in DWords
      8,             // Value

      ATTR_SOME_MASK,
      3,
      0x00FFFFFFFF, 0xFFFFFFFF, 0xFF000000,
  };

The attribute ids and meaning of the values will be documented in the
Programmer Reference Manuals when released.

Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Cc: Kenneth Graunke <kenneth.w.graunke@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Slawomir Milczarek <slawomir.milczarek@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
Reviewed-by: Matthew Brost <matthew.brost@intel.com>
Acked-by: Jordan Justen <jordan.l.justen@intel.com>
Tested-by: Jordan Justen <jordan.l.justen@intel.com>
Acked-by: Jon Bloomfield <jon.bloomfield@intel.com>
---
 drivers/gpu/drm/i915/i915_query.c | 23 +++++++++++++++++++++++
 include/uapi/drm/i915_drm.h       |  1 +
 2 files changed, 24 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_query.c b/drivers/gpu/drm/i915/i915_query.c
index 2dfbc22857a3..195524e9a369 100644
--- a/drivers/gpu/drm/i915/i915_query.c
+++ b/drivers/gpu/drm/i915/i915_query.c
@@ -479,12 +479,35 @@ static int query_memregion_info(struct drm_i915_private *i915,
 	return total_length;
 }
 
+static int query_hwconfig_blob(struct drm_i915_private *i915,
+			       struct drm_i915_query_item *query_item)
+{
+	struct intel_gt *gt = to_gt(i915);
+	struct intel_guc_hwconfig *hwconfig = &gt->uc.guc.hwconfig;
+
+	if (!hwconfig->size || !hwconfig->ptr)
+		return -ENODEV;
+
+	if (query_item->length == 0)
+		return hwconfig->size;
+
+	if (query_item->length < hwconfig->size)
+		return -EINVAL;
+
+	if (copy_to_user(u64_to_user_ptr(query_item->data_ptr),
+			 hwconfig->ptr, hwconfig->size))
+		return -EFAULT;
+
+	return hwconfig->size;
+}
+
 static int (* const i915_query_funcs[])(struct drm_i915_private *dev_priv,
 					struct drm_i915_query_item *query_item) = {
 	query_topology_info,
 	query_engine_info,
 	query_perf_config,
 	query_memregion_info,
+	query_hwconfig_blob,
 };
 
 int i915_query_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 914ebd9290e5..069d2fadfbd9 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -2685,6 +2685,7 @@ struct drm_i915_query_item {
 #define DRM_I915_QUERY_ENGINE_INFO	2
 #define DRM_I915_QUERY_PERF_CONFIG      3
 #define DRM_I915_QUERY_MEMORY_REGIONS   4
+#define DRM_I915_QUERY_HWCONFIG_BLOB	5
 /* Must be kept compact -- no holes and well documented */
 
 	/**
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread
* [Intel-gfx] [PATCH v5 2/4] drm/i915/uapi: Add query for hwconfig blob
@ 2022-02-22 10:36   ` Jordan Justen
  0 siblings, 0 replies; 32+ messages in thread
From: Jordan Justen @ 2022-02-22 10:36 UTC (permalink / raw)
  To: intel-gfx; +Cc: Kenneth Graunke, dri-devel, Slawomir Milczarek, Rodrigo Vivi

From: Rodrigo Vivi <rodrigo.vivi@intel.com>

The DRM_I915_QUERY_HWCONFIG_BLOB query item returns a blob of data
which it receives from the GuC software. This blob provides some
useful data about the hardware for drivers.

Although the blob is not fully documented at this time, the basic
format is an array of u32 values. The array is a simple and flexible
KLV (Key/Length/Value) formatted table. For example, it could be just:
enum device_attr { ATTR_SOME_VALUE = 0, ATTR_SOME_MASK = 1, };

  static const u32 hwconfig[] = {
      ATTR_SOME_VALUE,
      1,             // Value Length in DWords
      8,             // Value

      ATTR_SOME_MASK,
      3,
      0x00FFFFFFFF, 0xFFFFFFFF, 0xFF000000,
  };

The attribute ids and meaning of the values will be documented in the
Programmer Reference Manuals when released.

Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Cc: Kenneth Graunke <kenneth.w.graunke@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Slawomir Milczarek <slawomir.milczarek@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
Reviewed-by: Matthew Brost <matthew.brost@intel.com>
Acked-by: Jordan Justen <jordan.l.justen@intel.com>
Tested-by: Jordan Justen <jordan.l.justen@intel.com>
Acked-by: Jon Bloomfield <jon.bloomfield@intel.com>
---
 drivers/gpu/drm/i915/i915_query.c | 23 +++++++++++++++++++++++
 include/uapi/drm/i915_drm.h       |  1 +
 2 files changed, 24 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_query.c b/drivers/gpu/drm/i915/i915_query.c
index 2dfbc22857a3..195524e9a369 100644
--- a/drivers/gpu/drm/i915/i915_query.c
+++ b/drivers/gpu/drm/i915/i915_query.c
@@ -479,12 +479,35 @@ static int query_memregion_info(struct drm_i915_private *i915,
 	return total_length;
 }
 
+static int query_hwconfig_blob(struct drm_i915_private *i915,
+			       struct drm_i915_query_item *query_item)
+{
+	struct intel_gt *gt = to_gt(i915);
+	struct intel_guc_hwconfig *hwconfig = &gt->uc.guc.hwconfig;
+
+	if (!hwconfig->size || !hwconfig->ptr)
+		return -ENODEV;
+
+	if (query_item->length == 0)
+		return hwconfig->size;
+
+	if (query_item->length < hwconfig->size)
+		return -EINVAL;
+
+	if (copy_to_user(u64_to_user_ptr(query_item->data_ptr),
+			 hwconfig->ptr, hwconfig->size))
+		return -EFAULT;
+
+	return hwconfig->size;
+}
+
 static int (* const i915_query_funcs[])(struct drm_i915_private *dev_priv,
 					struct drm_i915_query_item *query_item) = {
 	query_topology_info,
 	query_engine_info,
 	query_perf_config,
 	query_memregion_info,
+	query_hwconfig_blob,
 };
 
 int i915_query_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 914ebd9290e5..069d2fadfbd9 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -2685,6 +2685,7 @@ struct drm_i915_query_item {
 #define DRM_I915_QUERY_ENGINE_INFO	2
 #define DRM_I915_QUERY_PERF_CONFIG      3
 #define DRM_I915_QUERY_MEMORY_REGIONS   4
+#define DRM_I915_QUERY_HWCONFIG_BLOB	5
 /* Must be kept compact -- no holes and well documented */
 
 	/**
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread
* [PATCH v5 3/4] drm/i915/uapi: Add struct drm_i915_query_hwconfig_blob_item
  2022-02-22 10:36 ` [Intel-gfx] " Jordan Justen
@ 2022-02-22 10:36   ` Jordan Justen
  -1 siblings, 0 replies; 32+ messages in thread
From: Jordan Justen @ 2022-02-22 10:36 UTC (permalink / raw)
  To: intel-gfx; +Cc: Jordan Justen, Jon Bloomfield, dri-devel, Daniel Vetter

Also, document DRM_I915_QUERY_HWCONFIG_BLOB with this struct.

v3:
 * Add various changes suggested by Tvrtko

v5:
 * Fix documenation formatting and verified with `make htmldocs` as
   suggested by Daniel

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
Acked-by: Jon Bloomfield <jon.bloomfield@intel.com>
Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 include/uapi/drm/i915_drm.h | 43 +++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 069d2fadfbd9..e44902ce8e64 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -3276,6 +3276,49 @@ struct drm_i915_gem_create_ext_protected_content {
 	__u32 flags;
 };
 
+/**
+ * DOC: GuC HWCONFIG blob uAPI
+ *
+ * The GuC produces a blob with information about the current device.
+ * i915 reads this blob from GuC and makes it available via this uAPI.
+ *
+ * The returned blob is a sequence of items of variable length
+ * described by struct drm_i915_query_hwconfig_blob_item.
+ *
+ * The overall blob returned by DRM_I915_QUERY_HWCONFIG_BLOB will end
+ * at the same location as the end of the final struct
+ * drm_i915_query_hwconfig_blob_item. In other words, walking through
+ * the individual items is guaranteed to eventually arrive at the
+ * exact end of the entire blob.
+ */
+
+/**
+ * struct drm_i915_query_hwconfig_blob_item - A single hwconfig item
+ * within the sequence of hwconfig items returned by
+ * DRM_I915_QUERY_HWCONFIG_BLOB.
+ *
+ * The length field gives the length of the data[] array. The length
+ * is the number of u32 items in the data[] array, and *not* the
+ * number of bytes.
+ *
+ * The key and length fields are required, so the minimum item size is
+ * 2 x u32, or 8 bytes, when the length field is 0. If the length
+ * field is 1, then the item's size is 12 bytes.
+ *
+ * The meaning of the key field and the data values are documented in
+ * the Programmer's Reference Manual.
+ */
+struct drm_i915_query_hwconfig_blob_item {
+	/** @key: Enum which defines how to interpret @data values. */
+	__u32 key;
+
+	/** @length: The number of u32 values in the @data array. */
+	__u32 length;
+
+	/** @data: Array of values with meaning defined by @key. */
+	__u32 data[];
+};
+
 /* ID of the protected content session managed by i915 when PXP is active */
 #define I915_PROTECTED_CONTENT_DEFAULT_SESSION 0xf
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread
* [Intel-gfx] [PATCH v5 3/4] drm/i915/uapi: Add struct drm_i915_query_hwconfig_blob_item
@ 2022-02-22 10:36   ` Jordan Justen
  0 siblings, 0 replies; 32+ messages in thread
From: Jordan Justen @ 2022-02-22 10:36 UTC (permalink / raw)
  To: intel-gfx; +Cc: dri-devel, Daniel Vetter

Also, document DRM_I915_QUERY_HWCONFIG_BLOB with this struct.

v3:
 * Add various changes suggested by Tvrtko

v5:
 * Fix documenation formatting and verified with `make htmldocs` as
   suggested by Daniel

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
Acked-by: Jon Bloomfield <jon.bloomfield@intel.com>
Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 include/uapi/drm/i915_drm.h | 43 +++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 069d2fadfbd9..e44902ce8e64 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -3276,6 +3276,49 @@ struct drm_i915_gem_create_ext_protected_content {
 	__u32 flags;
 };
 
+/**
+ * DOC: GuC HWCONFIG blob uAPI
+ *
+ * The GuC produces a blob with information about the current device.
+ * i915 reads this blob from GuC and makes it available via this uAPI.
+ *
+ * The returned blob is a sequence of items of variable length
+ * described by struct drm_i915_query_hwconfig_blob_item.
+ *
+ * The overall blob returned by DRM_I915_QUERY_HWCONFIG_BLOB will end
+ * at the same location as the end of the final struct
+ * drm_i915_query_hwconfig_blob_item. In other words, walking through
+ * the individual items is guaranteed to eventually arrive at the
+ * exact end of the entire blob.
+ */
+
+/**
+ * struct drm_i915_query_hwconfig_blob_item - A single hwconfig item
+ * within the sequence of hwconfig items returned by
+ * DRM_I915_QUERY_HWCONFIG_BLOB.
+ *
+ * The length field gives the length of the data[] array. The length
+ * is the number of u32 items in the data[] array, and *not* the
+ * number of bytes.
+ *
+ * The key and length fields are required, so the minimum item size is
+ * 2 x u32, or 8 bytes, when the length field is 0. If the length
+ * field is 1, then the item's size is 12 bytes.
+ *
+ * The meaning of the key field and the data values are documented in
+ * the Programmer's Reference Manual.
+ */
+struct drm_i915_query_hwconfig_blob_item {
+	/** @key: Enum which defines how to interpret @data values. */
+	__u32 key;
+
+	/** @length: The number of u32 values in the @data array. */
+	__u32 length;
+
+	/** @data: Array of values with meaning defined by @key. */
+	__u32 data[];
+};
+
 /* ID of the protected content session managed by i915 when PXP is active */
 #define I915_PROTECTED_CONTENT_DEFAULT_SESSION 0xf
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread
* [PATCH v5 4/4] drm/i915/guc: Verify hwconfig blob matches supported format
  2022-02-22 10:36 ` [Intel-gfx] " Jordan Justen
@ 2022-02-22 10:36   ` Jordan Justen
  -1 siblings, 0 replies; 32+ messages in thread
From: Jordan Justen @ 2022-02-22 10:36 UTC (permalink / raw)
  To: intel-gfx; +Cc: Jordan Justen, Jon Bloomfield, dri-devel

i915_drm.h now defines the format of the returned
DRM_I915_QUERY_HWCONFIG_BLOB query item. Since i915 receives this from
the black box GuC software, it should verify that the data matches
that format before sending it to user-space.

The verification makes a single simple pass through the blob contents,
so this verification step should not add a significant amount of init
time to i915.

v3:
 * Add various changes suggested by Tvrtko

v4:
 * Rewrite verify_hwconfig_blob() to hopefully be clearer without
   relying on comments so much, and add various suggestions from
   Michal.

Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
Acked-by: Jon Bloomfield <jon.bloomfield@intel.com>
---
 .../gpu/drm/i915/gt/uc/intel_guc_hwconfig.c   | 44 ++++++++++++++++++-
 1 file changed, 43 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c
index ad289603460c..a844b880cbdb 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c
@@ -73,9 +73,46 @@ static int guc_hwconfig_discover_size(struct intel_guc_hwconfig *hwconfig)
 	return 0;
 }
 
+static int verify_hwconfig_blob(struct intel_guc_hwconfig *hwconfig)
+{
+	struct intel_guc *guc = hwconfig_to_guc(hwconfig);
+	struct drm_device *drm = &guc_to_gt(guc)->i915->drm;
+	struct drm_i915_query_hwconfig_blob_item *item = hwconfig->ptr;
+	u64 offset = 0;
+	u64 remaining = hwconfig->size;
+	/* Everything before the data field is required */
+	u64 min_item_size = offsetof(struct drm_i915_query_hwconfig_blob_item, data);
+	u64 item_size;
+
+	if (!IS_ALIGNED(hwconfig->size, sizeof(u32))) {
+		drm_err(drm, "hwconfig blob size (%d) is not u32 aligned\n", hwconfig->size);
+		return -EINVAL;
+	}
+
+	while (offset < hwconfig->size) {
+		if (remaining < min_item_size) {
+			drm_err(drm, "hwconfig blob invalid (no room for item required fields at offset %lld)\n",
+				offset);
+			return -EINVAL;
+		}
+		item_size = min_item_size + sizeof(u32) * item->length;
+		if (item_size > remaining) {
+			drm_err(drm, "hwconfig blob invalid (no room for data array of item at offset %lld)\n",
+				offset);
+			return -EINVAL;
+		}
+		offset += item_size;
+		remaining -= item_size;
+		item = (void *)&item->data[item->length];
+	}
+
+	return 0;
+}
+
 static int guc_hwconfig_fill_buffer(struct intel_guc_hwconfig *hwconfig)
 {
 	struct intel_guc *guc = hwconfig_to_guc(hwconfig);
+	struct drm_device *drm = &guc_to_gt(guc)->i915->drm;
 	struct i915_vma *vma;
 	u32 ggtt_offset;
 	void *vaddr;
@@ -90,8 +127,13 @@ static int guc_hwconfig_fill_buffer(struct intel_guc_hwconfig *hwconfig)
 	ggtt_offset = intel_guc_ggtt_offset(guc, vma);
 
 	ret = __guc_action_get_hwconfig(guc, ggtt_offset, hwconfig->size);
-	if (ret >= 0)
+	if (ret >= 0) {
 		memcpy(hwconfig->ptr, vaddr, hwconfig->size);
+		if (verify_hwconfig_blob(hwconfig)) {
+			drm_err(drm, "Ignoring invalid hwconfig blob received from GuC!\n");
+			ret = -EINVAL;
+		}
+	}
 
 	i915_vma_unpin_and_release(&vma, I915_VMA_RELEASE_MAP);
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread
* [Intel-gfx] [PATCH v5 4/4] drm/i915/guc: Verify hwconfig blob matches supported format
@ 2022-02-22 10:36   ` Jordan Justen
  0 siblings, 0 replies; 32+ messages in thread
From: Jordan Justen @ 2022-02-22 10:36 UTC (permalink / raw)
  To: intel-gfx; +Cc: dri-devel

i915_drm.h now defines the format of the returned
DRM_I915_QUERY_HWCONFIG_BLOB query item. Since i915 receives this from
the black box GuC software, it should verify that the data matches
that format before sending it to user-space.

The verification makes a single simple pass through the blob contents,
so this verification step should not add a significant amount of init
time to i915.

v3:
 * Add various changes suggested by Tvrtko

v4:
 * Rewrite verify_hwconfig_blob() to hopefully be clearer without
   relying on comments so much, and add various suggestions from
   Michal.

Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
Acked-by: Jon Bloomfield <jon.bloomfield@intel.com>
---
 .../gpu/drm/i915/gt/uc/intel_guc_hwconfig.c   | 44 ++++++++++++++++++-
 1 file changed, 43 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c
index ad289603460c..a844b880cbdb 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c
@@ -73,9 +73,46 @@ static int guc_hwconfig_discover_size(struct intel_guc_hwconfig *hwconfig)
 	return 0;
 }
 
+static int verify_hwconfig_blob(struct intel_guc_hwconfig *hwconfig)
+{
+	struct intel_guc *guc = hwconfig_to_guc(hwconfig);
+	struct drm_device *drm = &guc_to_gt(guc)->i915->drm;
+	struct drm_i915_query_hwconfig_blob_item *item = hwconfig->ptr;
+	u64 offset = 0;
+	u64 remaining = hwconfig->size;
+	/* Everything before the data field is required */
+	u64 min_item_size = offsetof(struct drm_i915_query_hwconfig_blob_item, data);
+	u64 item_size;
+
+	if (!IS_ALIGNED(hwconfig->size, sizeof(u32))) {
+		drm_err(drm, "hwconfig blob size (%d) is not u32 aligned\n", hwconfig->size);
+		return -EINVAL;
+	}
+
+	while (offset < hwconfig->size) {
+		if (remaining < min_item_size) {
+			drm_err(drm, "hwconfig blob invalid (no room for item required fields at offset %lld)\n",
+				offset);
+			return -EINVAL;
+		}
+		item_size = min_item_size + sizeof(u32) * item->length;
+		if (item_size > remaining) {
+			drm_err(drm, "hwconfig blob invalid (no room for data array of item at offset %lld)\n",
+				offset);
+			return -EINVAL;
+		}
+		offset += item_size;
+		remaining -= item_size;
+		item = (void *)&item->data[item->length];
+	}
+
+	return 0;
+}
+
 static int guc_hwconfig_fill_buffer(struct intel_guc_hwconfig *hwconfig)
 {
 	struct intel_guc *guc = hwconfig_to_guc(hwconfig);
+	struct drm_device *drm = &guc_to_gt(guc)->i915->drm;
 	struct i915_vma *vma;
 	u32 ggtt_offset;
 	void *vaddr;
@@ -90,8 +127,13 @@ static int guc_hwconfig_fill_buffer(struct intel_guc_hwconfig *hwconfig)
 	ggtt_offset = intel_guc_ggtt_offset(guc, vma);
 
 	ret = __guc_action_get_hwconfig(guc, ggtt_offset, hwconfig->size);
-	if (ret >= 0)
+	if (ret >= 0) {
 		memcpy(hwconfig->ptr, vaddr, hwconfig->size);
+		if (verify_hwconfig_blob(hwconfig)) {
+			drm_err(drm, "Ignoring invalid hwconfig blob received from GuC!\n");
+			ret = -EINVAL;
+		}
+	}
 
 	i915_vma_unpin_and_release(&vma, I915_VMA_RELEASE_MAP);
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread
* Re: [Intel-gfx] [PATCH v5 4/4] drm/i915/guc: Verify hwconfig blob matches supported format
  2022-02-22 10:36   ` [Intel-gfx] " Jordan Justen
  (?)
@ 2022-02-22 11:24   ` Tvrtko Ursulin
  -1 siblings, 0 replies; 32+ messages in thread
From: Tvrtko Ursulin @ 2022-02-22 11:24 UTC (permalink / raw)
  To: Jordan Justen, intel-gfx; +Cc: dri-devel


On 22/02/2022 10:36, Jordan Justen wrote:
> i915_drm.h now defines the format of the returned
> DRM_I915_QUERY_HWCONFIG_BLOB query item. Since i915 receives this from
> the black box GuC software, it should verify that the data matches
> that format before sending it to user-space.
> 
> The verification makes a single simple pass through the blob contents,
> so this verification step should not add a significant amount of init
> time to i915.
> 
> v3:
>   * Add various changes suggested by Tvrtko
> 
> v4:
>   * Rewrite verify_hwconfig_blob() to hopefully be clearer without
>     relying on comments so much, and add various suggestions from
>     Michal.
> 
> Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
> Acked-by: Jon Bloomfield <jon.bloomfield@intel.com>
> ---
>   .../gpu/drm/i915/gt/uc/intel_guc_hwconfig.c   | 44 ++++++++++++++++++-
>   1 file changed, 43 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c
> index ad289603460c..a844b880cbdb 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c
> @@ -73,9 +73,46 @@ static int guc_hwconfig_discover_size(struct intel_guc_hwconfig *hwconfig)
>   	return 0;
>   }
>   
> +static int verify_hwconfig_blob(struct intel_guc_hwconfig *hwconfig)
> +{
> +	struct intel_guc *guc = hwconfig_to_guc(hwconfig);
> +	struct drm_device *drm = &guc_to_gt(guc)->i915->drm;
> +	struct drm_i915_query_hwconfig_blob_item *item = hwconfig->ptr;
> +	u64 offset = 0;
> +	u64 remaining = hwconfig->size;
> +	/* Everything before the data field is required */
> +	u64 min_item_size = offsetof(struct drm_i915_query_hwconfig_blob_item, data);
> +	u64 item_size;
> +
> +	if (!IS_ALIGNED(hwconfig->size, sizeof(u32))) {
> +		drm_err(drm, "hwconfig blob size (%d) is not u32 aligned\n", hwconfig->size);
> +		return -EINVAL;
> +	}
> +
> +	while (offset < hwconfig->size) {
> +		if (remaining < min_item_size) {
> +			drm_err(drm, "hwconfig blob invalid (no room for item required fields at offset %lld)\n",
> +				offset);
> +			return -EINVAL;
> +		}
> +		item_size = min_item_size + sizeof(u32) * item->length;
> +		if (item_size > remaining) {
> +			drm_err(drm, "hwconfig blob invalid (no room for data array of item at offset %lld)\n",
> +				offset);
> +			return -EINVAL;
> +		}
> +		offset += item_size;
> +		remaining -= item_size;
> +		item = (void *)&item->data[item->length];
> +	}
> +
> +	return 0;
> +}
> +
>   static int guc_hwconfig_fill_buffer(struct intel_guc_hwconfig *hwconfig)
>   {
>   	struct intel_guc *guc = hwconfig_to_guc(hwconfig);
> +	struct drm_device *drm = &guc_to_gt(guc)->i915->drm;
>   	struct i915_vma *vma;
>   	u32 ggtt_offset;
>   	void *vaddr;
> @@ -90,8 +127,13 @@ static int guc_hwconfig_fill_buffer(struct intel_guc_hwconfig *hwconfig)
>   	ggtt_offset = intel_guc_ggtt_offset(guc, vma);
>   
>   	ret = __guc_action_get_hwconfig(guc, ggtt_offset, hwconfig->size);
> -	if (ret >= 0)
> +	if (ret >= 0) {
>   		memcpy(hwconfig->ptr, vaddr, hwconfig->size);
> +		if (verify_hwconfig_blob(hwconfig)) {
> +			drm_err(drm, "Ignoring invalid hwconfig blob received from GuC!\n");
> +			ret = -EINVAL;
> +		}
> +	}
>   
>   	i915_vma_unpin_and_release(&vma, I915_VMA_RELEASE_MAP);
>   

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko

^ permalink raw reply	[flat|nested] 32+ messages in thread
* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for GuC HWCONFIG with documentation (rev5)
  2022-02-22 10:36 ` [Intel-gfx] " Jordan Justen
                   ` (4 preceding siblings ...)
  (?)
@ 2022-02-22 11:28 ` Patchwork
  -1 siblings, 0 replies; 32+ messages in thread
From: Patchwork @ 2022-02-22 11:28 UTC (permalink / raw)
  To: Jordan Justen; +Cc: intel-gfx

== Series Details ==

Series: GuC HWCONFIG with documentation (rev5)
URL   : https://patchwork.freedesktop.org/series/99787/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
c3d64e01c80f drm/i915/guc: Add fetch of hwconfig table
-:92: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating?
#92: 
new file mode 100644

total: 0 errors, 1 warnings, 0 checks, 236 lines checked
d5c4e6d3f58b drm/i915/uapi: Add query for hwconfig blob
2f033c0ee9f5 drm/i915/uapi: Add struct drm_i915_query_hwconfig_blob_item
5eee553b638e drm/i915/guc: Verify hwconfig blob matches supported format



^ permalink raw reply	[flat|nested] 32+ messages in thread
* [Intel-gfx] ✗ Fi.CI.SPARSE: warning for GuC HWCONFIG with documentation (rev5)
  2022-02-22 10:36 ` [Intel-gfx] " Jordan Justen
                   ` (5 preceding siblings ...)
  (?)
@ 2022-02-22 11:29 ` Patchwork
  -1 siblings, 0 replies; 32+ messages in thread
From: Patchwork @ 2022-02-22 11:29 UTC (permalink / raw)
  To: Jordan Justen; +Cc: intel-gfx

== Series Details ==

Series: GuC HWCONFIG with documentation (rev5)
URL   : https://patchwork.freedesktop.org/series/99787/
State : warning

== Summary ==

$ dim sparse --fast origin/drm-tip
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.



^ permalink raw reply	[flat|nested] 32+ messages in thread
* [Intel-gfx] ✓ Fi.CI.BAT: success for GuC HWCONFIG with documentation (rev5)
  2022-02-22 10:36 ` [Intel-gfx] " Jordan Justen
                   ` (6 preceding siblings ...)
  (?)
@ 2022-02-22 12:03 ` Patchwork
  2022-02-25  2:07   ` John Harrison
  -1 siblings, 1 reply; 32+ messages in thread
From: Patchwork @ 2022-02-22 12:03 UTC (permalink / raw)
  To: Jordan Justen; +Cc: intel-gfx

[-- Attachment #1: Type: text/plain, Size: 9341 bytes --]

== Series Details ==

Series: GuC HWCONFIG with documentation (rev5)
URL   : https://patchwork.freedesktop.org/series/99787/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_11264 -> Patchwork_22348
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/index.html

Participating hosts (44 -> 43)
------------------------------

  Additional (4): bat-dg2-8 fi-icl-u2 fi-bdw-5557u fi-pnv-d510 
  Missing    (5): fi-kbl-soraka shard-tglu fi-bsw-cyan shard-rkl shard-dg1 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_22348:

### IGT changes ###

#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@i915_hangman@error-state-basic:
    - {bat-adlp-6}:       [PASS][1] -> [DMESG-WARN][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11264/bat-adlp-6/igt@i915_hangman@error-state-basic.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/bat-adlp-6/igt@i915_hangman@error-state-basic.html

  * igt@i915_selftest@live:
    - {fi-tgl-dsi}:       NOTRUN -> [FAIL][3]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/fi-tgl-dsi/igt@i915_selftest@live.html

  
Known issues
------------

  Here are the changes found in Patchwork_22348 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@amdgpu/amd_cs_nop@fork-gfx0:
    - fi-icl-u2:          NOTRUN -> [SKIP][4] ([fdo#109315]) +17 similar issues
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/fi-icl-u2/igt@amdgpu/amd_cs_nop@fork-gfx0.html

  * igt@gem_exec_suspend@basic-s3@smem:
    - fi-bdw-5557u:       NOTRUN -> [INCOMPLETE][5] ([i915#146])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/fi-bdw-5557u/igt@gem_exec_suspend@basic-s3@smem.html

  * igt@gem_huc_copy@huc-copy:
    - fi-skl-6600u:       NOTRUN -> [SKIP][6] ([fdo#109271] / [i915#2190])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/fi-skl-6600u/igt@gem_huc_copy@huc-copy.html
    - fi-pnv-d510:        NOTRUN -> [SKIP][7] ([fdo#109271]) +39 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/fi-pnv-d510/igt@gem_huc_copy@huc-copy.html
    - fi-icl-u2:          NOTRUN -> [SKIP][8] ([i915#2190])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/fi-icl-u2/igt@gem_huc_copy@huc-copy.html

  * igt@gem_lmem_swapping@parallel-random-engines:
    - fi-icl-u2:          NOTRUN -> [SKIP][9] ([i915#4613]) +3 similar issues
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/fi-icl-u2/igt@gem_lmem_swapping@parallel-random-engines.html

  * igt@gem_lmem_swapping@verify-random:
    - fi-skl-6600u:       NOTRUN -> [SKIP][10] ([fdo#109271] / [i915#4613]) +3 similar issues
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/fi-skl-6600u/igt@gem_lmem_swapping@verify-random.html

  * igt@i915_selftest@live@requests:
    - fi-blb-e6850:       [PASS][11] -> [DMESG-FAIL][12] ([i915#5026])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11264/fi-blb-e6850/igt@i915_selftest@live@requests.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/fi-blb-e6850/igt@i915_selftest@live@requests.html
    - fi-pnv-d510:        NOTRUN -> [DMESG-FAIL][13] ([i915#2927])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/fi-pnv-d510/igt@i915_selftest@live@requests.html

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-icl-u2:          NOTRUN -> [SKIP][14] ([fdo#111827]) +8 similar issues
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/fi-icl-u2/igt@kms_chamelium@hdmi-hpd-fast.html

  * igt@kms_chamelium@vga-edid-read:
    - fi-skl-6600u:       NOTRUN -> [SKIP][15] ([fdo#109271] / [fdo#111827]) +8 similar issues
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/fi-skl-6600u/igt@kms_chamelium@vga-edid-read.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
    - fi-icl-u2:          NOTRUN -> [SKIP][16] ([fdo#109278]) +2 similar issues
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/fi-icl-u2/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy:
    - fi-skl-6600u:       NOTRUN -> [SKIP][17] ([fdo#109271]) +21 similar issues
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/fi-skl-6600u/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html

  * igt@kms_force_connector_basic@force-load-detect:
    - fi-icl-u2:          NOTRUN -> [SKIP][18] ([fdo#109285])
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/fi-icl-u2/igt@kms_force_connector_basic@force-load-detect.html

  * igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d:
    - fi-skl-6600u:       NOTRUN -> [SKIP][19] ([fdo#109271] / [i915#533])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/fi-skl-6600u/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d.html

  * igt@prime_vgem@basic-userptr:
    - fi-icl-u2:          NOTRUN -> [SKIP][20] ([i915#3301])
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/fi-icl-u2/igt@prime_vgem@basic-userptr.html

  * igt@runner@aborted:
    - fi-pnv-d510:        NOTRUN -> [FAIL][21] ([fdo#109271] / [i915#2403] / [i915#4312])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/fi-pnv-d510/igt@runner@aborted.html
    - fi-blb-e6850:       NOTRUN -> [FAIL][22] ([fdo#109271] / [i915#2403] / [i915#2426] / [i915#4312])
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/fi-blb-e6850/igt@runner@aborted.html

  
#### Possible fixes ####

  * igt@gem_exec_suspend@basic-s3@smem:
    - fi-skl-6600u:       [INCOMPLETE][23] ([i915#4547]) -> [PASS][24]
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11264/fi-skl-6600u/igt@gem_exec_suspend@basic-s3@smem.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/fi-skl-6600u/igt@gem_exec_suspend@basic-s3@smem.html

  * igt@i915_selftest@live@gt_pm:
    - fi-tgl-1115g4:      [DMESG-FAIL][25] ([i915#3987]) -> [PASS][26]
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11264/fi-tgl-1115g4/igt@i915_selftest@live@gt_pm.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/fi-tgl-1115g4/igt@i915_selftest@live@gt_pm.html

  
#### Warnings ####

  * igt@i915_selftest@live@hangcheck:
    - fi-hsw-4770:        [INCOMPLETE][27] ([i915#3303]) -> [INCOMPLETE][28] ([i915#4785])
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11264/fi-hsw-4770/igt@i915_selftest@live@hangcheck.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/fi-hsw-4770/igt@i915_selftest@live@hangcheck.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#1436]: https://gitlab.freedesktop.org/drm/intel/issues/1436
  [i915#146]: https://gitlab.freedesktop.org/drm/intel/issues/146
  [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#2403]: https://gitlab.freedesktop.org/drm/intel/issues/2403
  [i915#2426]: https://gitlab.freedesktop.org/drm/intel/issues/2426
  [i915#2927]: https://gitlab.freedesktop.org/drm/intel/issues/2927
  [i915#3301]: https://gitlab.freedesktop.org/drm/intel/issues/3301
  [i915#3303]: https://gitlab.freedesktop.org/drm/intel/issues/3303
  [i915#3987]: https://gitlab.freedesktop.org/drm/intel/issues/3987
  [i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
  [i915#4547]: https://gitlab.freedesktop.org/drm/intel/issues/4547
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#4785]: https://gitlab.freedesktop.org/drm/intel/issues/4785
  [i915#5026]: https://gitlab.freedesktop.org/drm/intel/issues/5026
  [i915#5127]: https://gitlab.freedesktop.org/drm/intel/issues/5127
  [i915#5137]: https://gitlab.freedesktop.org/drm/intel/issues/5137
  [i915#533]: https://gitlab.freedesktop.org/drm/intel/issues/533


Build changes
-------------

  * Linux: CI_DRM_11264 -> Patchwork_22348

  CI-20190529: 20190529
  CI_DRM_11264: 3b562232559bd940e63f977619548e5cc70da985 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_6349: 0513032006f385f34fcd094c810b93f31cbed09d @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_22348: 5eee553b638ed3f343e129636be5fade999534e8 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

5eee553b638e drm/i915/guc: Verify hwconfig blob matches supported format
2f033c0ee9f5 drm/i915/uapi: Add struct drm_i915_query_hwconfig_blob_item
d5c4e6d3f58b drm/i915/uapi: Add query for hwconfig blob
c3d64e01c80f drm/i915/guc: Add fetch of hwconfig table

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/index.html

[-- Attachment #2: Type: text/html, Size: 11114 bytes --]

^ permalink raw reply	[flat|nested] 32+ messages in thread
* [Intel-gfx] ✓ Fi.CI.IGT: success for GuC HWCONFIG with documentation (rev5)
  2022-02-22 10:36 ` [Intel-gfx] " Jordan Justen
                   ` (7 preceding siblings ...)
  (?)
@ 2022-02-22 13:30 ` Patchwork
  -1 siblings, 0 replies; 32+ messages in thread
From: Patchwork @ 2022-02-22 13:30 UTC (permalink / raw)
  To: Jordan Justen; +Cc: intel-gfx

[-- Attachment #1: Type: text/plain, Size: 30261 bytes --]

== Series Details ==

Series: GuC HWCONFIG with documentation (rev5)
URL   : https://patchwork.freedesktop.org/series/99787/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_11264_full -> Patchwork_22348_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Participating hosts (13 -> 13)
------------------------------

  No changes in participating hosts

Known issues
------------

  Here are the changes found in Patchwork_22348_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_eio@in-flight-contexts-10ms:
    - shard-tglb:         [PASS][1] -> [TIMEOUT][2] ([i915#3063])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11264/shard-tglb3/igt@gem_eio@in-flight-contexts-10ms.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-tglb5/igt@gem_eio@in-flight-contexts-10ms.html

  * igt@gem_exec_balancer@parallel-ordering:
    - shard-kbl:          NOTRUN -> [DMESG-FAIL][3] ([i915#5076])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-kbl6/igt@gem_exec_balancer@parallel-ordering.html

  * igt@gem_exec_capture@pi@bcs0:
    - shard-skl:          NOTRUN -> [INCOMPLETE][4] ([i915#4547])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-skl9/igt@gem_exec_capture@pi@bcs0.html

  * igt@gem_exec_fair@basic-flow@rcs0:
    - shard-tglb:         [PASS][5] -> [FAIL][6] ([i915#2842])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11264/shard-tglb3/igt@gem_exec_fair@basic-flow@rcs0.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-tglb1/igt@gem_exec_fair@basic-flow@rcs0.html

  * igt@gem_exec_fair@basic-none@vcs0:
    - shard-kbl:          [PASS][7] -> [FAIL][8] ([i915#2842]) +3 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11264/shard-kbl4/igt@gem_exec_fair@basic-none@vcs0.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-kbl4/igt@gem_exec_fair@basic-none@vcs0.html

  * igt@gem_exec_fair@basic-pace@rcs0:
    - shard-iclb:         [PASS][9] -> [FAIL][10] ([i915#2842]) +1 similar issue
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11264/shard-iclb7/igt@gem_exec_fair@basic-pace@rcs0.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-iclb1/igt@gem_exec_fair@basic-pace@rcs0.html

  * igt@gem_exec_fair@basic-pace@vcs1:
    - shard-iclb:         NOTRUN -> [FAIL][11] ([i915#2842])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-iclb1/igt@gem_exec_fair@basic-pace@vcs1.html

  * igt@gem_exec_parallel@engines@basic:
    - shard-glk:          [PASS][12] -> [DMESG-WARN][13] ([i915#118]) +1 similar issue
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11264/shard-glk2/igt@gem_exec_parallel@engines@basic.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-glk4/igt@gem_exec_parallel@engines@basic.html

  * igt@gem_lmem_swapping@heavy-multi:
    - shard-skl:          NOTRUN -> [SKIP][14] ([fdo#109271] / [i915#4613]) +1 similar issue
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-skl1/igt@gem_lmem_swapping@heavy-multi.html

  * igt@gem_lmem_swapping@heavy-verify-random:
    - shard-kbl:          NOTRUN -> [SKIP][15] ([fdo#109271] / [i915#4613]) +2 similar issues
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-kbl6/igt@gem_lmem_swapping@heavy-verify-random.html

  * igt@gem_lmem_swapping@smem-oom:
    - shard-apl:          NOTRUN -> [SKIP][16] ([fdo#109271] / [i915#4613])
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-apl8/igt@gem_lmem_swapping@smem-oom.html

  * igt@gem_pwrite@basic-exhaustion:
    - shard-skl:          NOTRUN -> [WARN][17] ([i915#2658])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-skl9/igt@gem_pwrite@basic-exhaustion.html

  * igt@gem_pxp@reject-modify-context-protection-off-1:
    - shard-iclb:         NOTRUN -> [SKIP][18] ([i915#4270])
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-iclb8/igt@gem_pxp@reject-modify-context-protection-off-1.html

  * igt@gem_render_copy@y-tiled-mc-ccs-to-vebox-y-tiled:
    - shard-iclb:         NOTRUN -> [SKIP][19] ([i915#768]) +2 similar issues
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-iclb7/igt@gem_render_copy@y-tiled-mc-ccs-to-vebox-y-tiled.html

  * igt@gem_userptr_blits@dmabuf-sync:
    - shard-kbl:          NOTRUN -> [SKIP][20] ([fdo#109271] / [i915#3323])
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-kbl6/igt@gem_userptr_blits@dmabuf-sync.html

  * igt@i915_pm_dc@dc6-dpms:
    - shard-kbl:          NOTRUN -> [FAIL][21] ([i915#454])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-kbl6/igt@i915_pm_dc@dc6-dpms.html
    - shard-skl:          NOTRUN -> [FAIL][22] ([i915#454])
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-skl1/igt@i915_pm_dc@dc6-dpms.html

  * igt@i915_pm_rpm@gem-execbuf-stress-pc8:
    - shard-iclb:         NOTRUN -> [SKIP][23] ([fdo#109293] / [fdo#109506])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-iclb3/igt@i915_pm_rpm@gem-execbuf-stress-pc8.html

  * igt@i915_pm_sseu@full-enable:
    - shard-iclb:         NOTRUN -> [SKIP][24] ([i915#4387])
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-iclb3/igt@i915_pm_sseu@full-enable.html

  * igt@i915_selftest@mock@hugepages:
    - shard-skl:          NOTRUN -> [INCOMPLETE][25] ([i915#5123])
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-skl3/igt@i915_selftest@mock@hugepages.html

  * igt@kms_big_fb@x-tiled-max-hw-stride-32bpp-rotate-180-hflip-async-flip:
    - shard-kbl:          NOTRUN -> [SKIP][26] ([fdo#109271] / [i915#3777])
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-kbl6/igt@kms_big_fb@x-tiled-max-hw-stride-32bpp-rotate-180-hflip-async-flip.html

  * igt@kms_big_fb@y-tiled-max-hw-stride-64bpp-rotate-0-hflip-async-flip:
    - shard-apl:          NOTRUN -> [SKIP][27] ([fdo#109271] / [i915#3777]) +1 similar issue
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-apl8/igt@kms_big_fb@y-tiled-max-hw-stride-64bpp-rotate-0-hflip-async-flip.html

  * igt@kms_big_fb@yf-tiled-8bpp-rotate-0:
    - shard-iclb:         NOTRUN -> [SKIP][28] ([fdo#110723])
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-iclb7/igt@kms_big_fb@yf-tiled-8bpp-rotate-0.html

  * igt@kms_big_fb@yf-tiled-max-hw-stride-32bpp-rotate-180-hflip:
    - shard-skl:          NOTRUN -> [SKIP][29] ([fdo#109271] / [i915#3777])
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-skl9/igt@kms_big_fb@yf-tiled-max-hw-stride-32bpp-rotate-180-hflip.html

  * igt@kms_big_fb@yf-tiled-max-hw-stride-64bpp-rotate-0:
    - shard-apl:          NOTRUN -> [SKIP][30] ([fdo#109271]) +113 similar issues
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-apl3/igt@kms_big_fb@yf-tiled-max-hw-stride-64bpp-rotate-0.html

  * igt@kms_ccs@pipe-b-missing-ccs-buffer-y_tiled_gen12_rc_ccs_cc:
    - shard-apl:          NOTRUN -> [SKIP][31] ([fdo#109271] / [i915#3886]) +4 similar issues
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-apl3/igt@kms_ccs@pipe-b-missing-ccs-buffer-y_tiled_gen12_rc_ccs_cc.html

  * igt@kms_ccs@pipe-b-random-ccs-data-y_tiled_gen12_mc_ccs:
    - shard-iclb:         NOTRUN -> [SKIP][32] ([fdo#109278] / [i915#3886]) +2 similar issues
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-iclb7/igt@kms_ccs@pipe-b-random-ccs-data-y_tiled_gen12_mc_ccs.html

  * igt@kms_ccs@pipe-c-crc-primary-rotation-180-y_tiled_gen12_rc_ccs_cc:
    - shard-kbl:          NOTRUN -> [SKIP][33] ([fdo#109271] / [i915#3886]) +4 similar issues
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-kbl6/igt@kms_ccs@pipe-c-crc-primary-rotation-180-y_tiled_gen12_rc_ccs_cc.html

  * igt@kms_ccs@pipe-c-missing-ccs-buffer-y_tiled_gen12_rc_ccs_cc:
    - shard-skl:          NOTRUN -> [SKIP][34] ([fdo#109271] / [i915#3886]) +7 similar issues
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-skl9/igt@kms_ccs@pipe-c-missing-ccs-buffer-y_tiled_gen12_rc_ccs_cc.html

  * igt@kms_chamelium@dp-frame-dump:
    - shard-kbl:          NOTRUN -> [SKIP][35] ([fdo#109271] / [fdo#111827]) +6 similar issues
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-kbl7/igt@kms_chamelium@dp-frame-dump.html

  * igt@kms_color_chamelium@pipe-a-ctm-0-5:
    - shard-apl:          NOTRUN -> [SKIP][36] ([fdo#109271] / [fdo#111827]) +7 similar issues
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-apl7/igt@kms_color_chamelium@pipe-a-ctm-0-5.html

  * igt@kms_color_chamelium@pipe-a-ctm-green-to-red:
    - shard-skl:          NOTRUN -> [SKIP][37] ([fdo#109271] / [fdo#111827]) +7 similar issues
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-skl9/igt@kms_color_chamelium@pipe-a-ctm-green-to-red.html

  * igt@kms_color_chamelium@pipe-c-ctm-0-5:
    - shard-iclb:         NOTRUN -> [SKIP][38] ([fdo#109284] / [fdo#111827]) +2 similar issues
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-iclb3/igt@kms_color_chamelium@pipe-c-ctm-0-5.html

  * igt@kms_cursor_crc@pipe-d-cursor-32x32-rapid-movement:
    - shard-iclb:         NOTRUN -> [SKIP][39] ([fdo#109278]) +10 similar issues
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-iclb7/igt@kms_cursor_crc@pipe-d-cursor-32x32-rapid-movement.html

  * igt@kms_cursor_legacy@flip-vs-cursor-varying-size:
    - shard-iclb:         [PASS][40] -> [FAIL][41] ([i915#2346])
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11264/shard-iclb8/igt@kms_cursor_legacy@flip-vs-cursor-varying-size.html
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-iclb7/igt@kms_cursor_legacy@flip-vs-cursor-varying-size.html

  * igt@kms_cursor_legacy@pipe-d-single-bo:
    - shard-skl:          NOTRUN -> [SKIP][42] ([fdo#109271] / [i915#533])
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-skl9/igt@kms_cursor_legacy@pipe-d-single-bo.html

  * igt@kms_cursor_legacy@pipe-d-torture-bo:
    - shard-apl:          NOTRUN -> [SKIP][43] ([fdo#109271] / [i915#533]) +2 similar issues
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-apl7/igt@kms_cursor_legacy@pipe-d-torture-bo.html

  * igt@kms_flip@2x-plain-flip-fb-recreate:
    - shard-iclb:         NOTRUN -> [SKIP][44] ([fdo#109274])
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-iclb7/igt@kms_flip@2x-plain-flip-fb-recreate.html

  * igt@kms_flip@flip-vs-expired-vblank@a-edp1:
    - shard-skl:          [PASS][45] -> [FAIL][46] ([i915#79])
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11264/shard-skl10/igt@kms_flip@flip-vs-expired-vblank@a-edp1.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-skl2/igt@kms_flip@flip-vs-expired-vblank@a-edp1.html

  * igt@kms_flip@flip-vs-suspend-interruptible@a-dp1:
    - shard-apl:          [PASS][47] -> [DMESG-WARN][48] ([i915#180]) +2 similar issues
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11264/shard-apl1/igt@kms_flip@flip-vs-suspend-interruptible@a-dp1.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-apl2/igt@kms_flip@flip-vs-suspend-interruptible@a-dp1.html

  * igt@kms_flip@flip-vs-suspend@b-dp1:
    - shard-apl:          NOTRUN -> [DMESG-WARN][49] ([i915#180])
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-apl3/igt@kms_flip@flip-vs-suspend@b-dp1.html

  * igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-32bpp-ytileccs-downscaling:
    - shard-iclb:         [PASS][50] -> [SKIP][51] ([i915#3701])
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11264/shard-iclb8/igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-32bpp-ytileccs-downscaling.html
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-iclb2/igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-32bpp-ytileccs-downscaling.html

  * igt@kms_frontbuffer_tracking@fbc-1p-shrfb-fliptrack-mmap-gtt:
    - shard-skl:          NOTRUN -> [SKIP][52] ([fdo#109271]) +91 similar issues
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-skl1/igt@kms_frontbuffer_tracking@fbc-1p-shrfb-fliptrack-mmap-gtt.html

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-pri-indfb-draw-mmap-gtt:
    - shard-iclb:         NOTRUN -> [SKIP][53] ([fdo#109280]) +6 similar issues
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-iclb7/igt@kms_frontbuffer_tracking@fbc-2p-primscrn-pri-indfb-draw-mmap-gtt.html

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-blt:
    - shard-kbl:          NOTRUN -> [SKIP][54] ([fdo#109271]) +89 similar issues
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-kbl7/igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-blt.html

  * igt@kms_hdr@bpc-switch:
    - shard-skl:          [PASS][55] -> [FAIL][56] ([i915#1188])
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11264/shard-skl7/igt@kms_hdr@bpc-switch.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-skl4/igt@kms_hdr@bpc-switch.html

  * igt@kms_hdr@bpc-switch-suspend:
    - shard-kbl:          [PASS][57] -> [DMESG-WARN][58] ([i915#180]) +3 similar issues
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11264/shard-kbl3/igt@kms_hdr@bpc-switch-suspend.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-kbl4/igt@kms_hdr@bpc-switch-suspend.html

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-d-frame-sequence:
    - shard-kbl:          NOTRUN -> [SKIP][59] ([fdo#109271] / [i915#533])
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-kbl7/igt@kms_pipe_crc_basic@nonblocking-crc-pipe-d-frame-sequence.html

  * igt@kms_plane_alpha_blend@pipe-a-alpha-transparent-fb:
    - shard-skl:          NOTRUN -> [FAIL][60] ([i915#265])
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-skl9/igt@kms_plane_alpha_blend@pipe-a-alpha-transparent-fb.html

  * igt@kms_plane_alpha_blend@pipe-b-constant-alpha-max:
    - shard-apl:          NOTRUN -> [FAIL][61] ([fdo#108145] / [i915#265]) +1 similar issue
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-apl7/igt@kms_plane_alpha_blend@pipe-b-constant-alpha-max.html

  * igt@kms_plane_alpha_blend@pipe-c-alpha-7efc:
    - shard-kbl:          NOTRUN -> [FAIL][62] ([fdo#108145] / [i915#265]) +1 similar issue
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-kbl7/igt@kms_plane_alpha_blend@pipe-c-alpha-7efc.html

  * igt@kms_plane_alpha_blend@pipe-c-constant-alpha-max:
    - shard-skl:          NOTRUN -> [FAIL][63] ([fdo#108145] / [i915#265])
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-skl1/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-max.html

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          [PASS][64] -> [FAIL][65] ([fdo#108145] / [i915#265])
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11264/shard-skl1/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-skl1/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html

  * igt@kms_plane_lowres@pipe-a-tiling-x:
    - shard-iclb:         NOTRUN -> [SKIP][66] ([i915#3536]) +2 similar issues
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-iclb6/igt@kms_plane_lowres@pipe-a-tiling-x.html

  * igt@kms_plane_scaling@scaler-with-clipping-clamping@pipe-c-scaler-with-clipping-clamping:
    - shard-skl:          NOTRUN -> [SKIP][67] ([fdo#109271] / [i915#2733])
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-skl9/igt@kms_plane_scaling@scaler-with-clipping-clamping@pipe-c-scaler-with-clipping-clamping.html

  * igt@kms_psr2_sf@primary-plane-update-sf-dmg-area:
    - shard-iclb:         NOTRUN -> [SKIP][68] ([fdo#111068] / [i915#658])
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-iclb7/igt@kms_psr2_sf@primary-plane-update-sf-dmg-area.html
    - shard-skl:          NOTRUN -> [SKIP][69] ([fdo#109271] / [i915#658]) +1 similar issue
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-skl1/igt@kms_psr2_sf@primary-plane-update-sf-dmg-area.html

  * igt@kms_psr2_su@page_flip-xrgb8888:
    - shard-kbl:          NOTRUN -> [SKIP][70] ([fdo#109271] / [i915#658])
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-kbl7/igt@kms_psr2_su@page_flip-xrgb8888.html

  * igt@kms_psr@psr2_sprite_plane_move:
    - shard-iclb:         [PASS][71] -> [SKIP][72] ([fdo#109441]) +1 similar issue
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11264/shard-iclb2/igt@kms_psr@psr2_sprite_plane_move.html
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-iclb8/igt@kms_psr@psr2_sprite_plane_move.html

  * igt@kms_setmode@basic:
    - shard-glk:          [PASS][73] -> [FAIL][74] ([i915#31])
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11264/shard-glk5/igt@kms_setmode@basic.html
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-glk9/igt@kms_setmode@basic.html

  * igt@kms_vblank@pipe-b-ts-continuation-suspend:
    - shard-kbl:          NOTRUN -> [DMESG-WARN][75] ([i915#180])
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-kbl6/igt@kms_vblank@pipe-b-ts-continuation-suspend.html

  * igt@kms_writeback@writeback-pixel-formats:
    - shard-skl:          NOTRUN -> [SKIP][76] ([fdo#109271] / [i915#2437])
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-skl9/igt@kms_writeback@writeback-pixel-formats.html

  * igt@nouveau_crc@ctx-flip-threshold-reset-after-capture:
    - shard-iclb:         NOTRUN -> [SKIP][77] ([i915#2530]) +2 similar issues
   [77]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-iclb3/igt@nouveau_crc@ctx-flip-threshold-reset-after-capture.html

  * igt@perf@per-context-mode-unprivileged:
    - shard-iclb:         NOTRUN -> [SKIP][78] ([fdo#109289]) +1 similar issue
   [78]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-iclb3/igt@perf@per-context-mode-unprivileged.html

  * igt@perf@polling:
    - shard-skl:          NOTRUN -> [FAIL][79] ([i915#1542])
   [79]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-skl1/igt@perf@polling.html

  * igt@perf_pmu@module-unload:
    - shard-iclb:         NOTRUN -> [FAIL][80] ([i915#5136])
   [80]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-iclb7/igt@perf_pmu@module-unload.html
    - shard-skl:          NOTRUN -> [FAIL][81] ([i915#5136])
   [81]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-skl1/igt@perf_pmu@module-unload.html

  * igt@prime_nv_pcopy@test1_macro:
    - shard-tglb:         NOTRUN -> [SKIP][82] ([fdo#109291])
   [82]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-tglb6/igt@prime_nv_pcopy@test1_macro.html
    - shard-iclb:         NOTRUN -> [SKIP][83] ([fdo#109291])
   [83]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-iclb6/igt@prime_nv_pcopy@test1_macro.html

  * igt@sysfs_clients@create:
    - shard-apl:          NOTRUN -> [SKIP][84] ([fdo#109271] / [i915#2994]) +1 similar issue
   [84]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-apl3/igt@sysfs_clients@create.html

  
#### Possible fixes ####

  * igt@fbdev@nullptr:
    - {shard-rkl}:        [SKIP][85] ([i915#2582]) -> [PASS][86]
   [85]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11264/shard-rkl-1/igt@fbdev@nullptr.html
   [86]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-rkl-6/igt@fbdev@nullptr.html

  * igt@gem_exec_capture@pi@rcs0:
    - shard-skl:          [INCOMPLETE][87] ([i915#4547]) -> [PASS][88]
   [87]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11264/shard-skl4/igt@gem_exec_capture@pi@rcs0.html
   [88]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-skl9/igt@gem_exec_capture@pi@rcs0.html

  * igt@gem_exec_capture@pi@vcs0:
    - {shard-tglu}:       [INCOMPLETE][89] ([i915#3371]) -> [PASS][90]
   [89]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11264/shard-tglu-3/igt@gem_exec_capture@pi@vcs0.html
   [90]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-tglu-6/igt@gem_exec_capture@pi@vcs0.html

  * igt@gem_exec_fair@basic-throttle@rcs0:
    - shard-iclb:         [FAIL][91] ([i915#2842]) -> [PASS][92]
   [91]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11264/shard-iclb5/igt@gem_exec_fair@basic-throttle@rcs0.html
   [92]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-iclb5/igt@gem_exec_fair@basic-throttle@rcs0.html

  * igt@i915_pm_backlight@fade_with_dpms:
    - {shard-rkl}:        [SKIP][93] ([i915#3012]) -> [PASS][94]
   [93]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11264/shard-rkl-2/igt@i915_pm_backlight@fade_with_dpms.html
   [94]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-rkl-6/igt@i915_pm_backlight@fade_with_dpms.html

  * igt@i915_pm_rpm@cursor:
    - {shard-rkl}:        ([SKIP][95], [SKIP][96]) ([i915#1849]) -> [PASS][97]
   [95]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11264/shard-rkl-1/igt@i915_pm_rpm@cursor.html
   [96]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11264/shard-rkl-4/igt@i915_pm_rpm@cursor.html
   [97]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-rkl-6/igt@i915_pm_rpm@cursor.html

  * igt@i915_pm_rpm@drm-resources-equal:
    - {shard-rkl}:        [SKIP][98] ([fdo#109308]) -> [PASS][99]
   [98]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11264/shard-rkl-2/igt@i915_pm_rpm@drm-resources-equal.html
   [99]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-rkl-6/igt@i915_pm_rpm@drm-resources-equal.html

  * igt@kms_big_fb@linear-16bpp-rotate-180:
    - {shard-tglu}:       [DMESG-WARN][100] ([i915#402]) -> [PASS][101]
   [100]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11264/shard-tglu-4/igt@kms_big_fb@linear-16bpp-rotate-180.html
   [101]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-tglu-8/igt@kms_big_fb@linear-16bpp-rotate-180.html

  * igt@kms_big_fb@x-tiled-max-hw-stride-32bpp-rotate-0-hflip-async-flip:
    - {shard-rkl}:        [SKIP][102] ([i915#1845]) -> [PASS][103] +11 similar issues
   [102]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11264/shard-rkl-2/igt@kms_big_fb@x-tiled-max-hw-stride-32bpp-rotate-0-hflip-async-flip.html
   [103]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-rkl-6/igt@kms_big_fb@x-tiled-max-hw-stride-32bpp-rotate-0-hflip-async-flip.html

  * igt@kms_big_fb@x-tiled-max-hw-stride-64bpp-rotate-180-hflip:
    - {shard-rkl}:        ([SKIP][104], [SKIP][105]) ([i915#1845]) -> [PASS][106]
   [104]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11264/shard-rkl-4/igt@kms_big_fb@x-tiled-max-hw-stride-64bpp-rotate-180-hflip.html
   [105]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11264/shard-rkl-2/igt@kms_big_fb@x-tiled-max-hw-stride-64bpp-rotate-180-hflip.html
   [106]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-rkl-6/igt@kms_big_fb@x-tiled-max-hw-stride-64bpp-rotate-180-hflip.html

  * igt@kms_ccs@pipe-a-crc-sprite-planes-basic-y_tiled_gen12_rc_ccs:
    - {shard-rkl}:        [SKIP][107] ([i915#1845] / [i915#4098]) -> [PASS][108] +2 similar issues
   [107]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11264/shard-rkl-1/igt@kms_ccs@pipe-a-crc-sprite-planes-basic-y_tiled_gen12_rc_ccs.html
   [108]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-rkl-6/igt@kms_ccs@pipe-a-crc-sprite-planes-basic-y_tiled_gen12_rc_ccs.html

  * igt@kms_color@pipe-a-ctm-green-to-red:
    - {shard-rkl}:        [SKIP][109] ([i915#1149] / [i915#1849] / [i915#4070]) -> [PASS][110]
   [109]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11264/shard-rkl-1/igt@kms_color@pipe-a-ctm-green-to-red.html
   [110]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-rkl-6/igt@kms_color@pipe-a-ctm-green-to-red.html

  * igt@kms_cursor_crc@pipe-b-cursor-64x64-random:
    - {shard-rkl}:        [SKIP][111] ([fdo#112022] / [i915#4070]) -> [PASS][112] +8 similar issues
   [111]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11264/shard-rkl-2/igt@kms_cursor_crc@pipe-b-cursor-64x64-random.html
   [112]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-rkl-6/igt@kms_cursor_crc@pipe-b-cursor-64x64-random.html

  * igt@kms_cursor_crc@pipe-c-cursor-suspend:
    - shard-skl:          [INCOMPLETE][113] ([i915#300]) -> [PASS][114]
   [113]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11264/shard-skl1/igt@kms_cursor_crc@pipe-c-cursor-suspend.html
   [114]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-skl1/igt@kms_cursor_crc@pipe-c-cursor-suspend.html

  * igt@kms_cursor_legacy@basic-flip-after-cursor-varying-size:
    - {shard-rkl}:        ([SKIP][115], [SKIP][116]) ([fdo#111825] / [i915#4070]) -> [PASS][117]
   [115]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11264/shard-rkl-2/igt@kms_cursor_legacy@basic-flip-after-cursor-varying-size.html
   [116]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11264/shard-rkl-4/igt@kms_cursor_legacy@basic-flip-after-cursor-varying-size.html
   [117]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-rkl-6/igt@kms_cursor_legacy@basic-flip-after-cursor-varying-size.html

  * igt@kms_cursor_legacy@cursor-vs-flip-atomic:
    - {shard-rkl}:        [SKIP][118] ([fdo#111825] / [i915#4070]) -> [PASS][119] +3 similar issues
   [118]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11264/shard-rkl-2/igt@kms_cursor_legacy@cursor-vs-flip-atomic.html
   [119]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-rkl-6/igt@kms_cursor_legacy@cursor-vs-flip-atomic.html

  * igt@kms_cursor_legacy@pipe-c-forked-move:
    - {shard-rkl}:        [SKIP][120] ([i915#4070]) -> [PASS][121]
   [120]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11264/shard-rkl-1/igt@kms_cursor_legacy@pipe-c-forked-move.html
   [121]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-rkl-4/igt@kms_cursor_legacy@pipe-c-forked-move.html

  * igt@kms_draw_crc@draw-method-xrgb2101010-mmap-gtt-xtiled:
    - {shard-rkl}:        [SKIP][122] ([fdo#111314]) -> [PASS][123] +7 similar issues
   [122]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11264/shard-rkl-2/igt@kms_draw_crc@draw-method-xrgb2101010-mmap-gtt-xtiled.html
   [123]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-rkl-6/igt@kms_draw_crc@draw-method-xrgb2101010-mmap-gtt-xtiled.html

  * igt@kms_flip@flip-vs-expired-vblank@c-edp1:
    - shard-skl:          [FAIL][124] ([i915#79]) -> [PASS][125]
   [124]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11264/shard-skl10/igt@kms_flip@flip-vs-expired-vblank@c-edp1.html
   [125]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-skl2/igt@kms_flip@flip-vs-expired-vblank@c-edp1.html

  * igt@kms_flip@flip-vs-suspend-interruptible@a-dp1:
    - shard-kbl:          [DMESG-WARN][126] ([i915#180]) -> [PASS][127] +4 similar issues
   [126]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11264/shard-kbl6/igt@kms_flip@flip-vs-suspend-interruptible@a-dp1.html
   [127]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-kbl3/igt@kms_flip@flip-vs-suspend-interruptible@a-dp1.html

  * igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-64bpp-ytile-upscaling:
    - shard-glk:          [FAIL][128] ([i915#4911]) -> [PASS][129]
   [128]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11264/shard-glk8/igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-64bpp-ytile-upscaling.html
   [129]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-glk9/igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-64bpp-ytile-upscaling.html

  * igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-render:
    - {shard-rkl}:        [SKIP][130] ([i915#1849]) -> [PASS][131] +23 similar issues
   [130]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11264/shard-rkl-2/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-render.html
   [131]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-rkl-6/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-render.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-blt:
    - {shard-rkl}:        ([SKIP][132], [SKIP][133]) ([i915#1849] / [i915#4098]) -> [PASS][134]
   [132]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11264/shard-rkl-4/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-blt.html
   [133]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11264/shard-rkl-2/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-blt.html
   [134]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-rkl-6/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-blt.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-mmap-cpu:
    - {shard-rkl}:        ([SKIP][135], [PASS][136]) ([i915#4098]) -> [PASS][137]
   [135]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11264/shard-rkl-4/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-mmap-cpu.html
   [136]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11264/shard-rkl-6/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-mmap-cpu.html
   [137]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-rkl-6/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-mmap-cpu.html

  * igt@kms_plane@plane-panning-bottom-right-suspend@pipe-a-planes:
    - shard-apl:          [DMESG-WARN][138] ([i915#180]) -> [PASS][139] +3 similar issues
   [138]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11264/shard-apl6/igt@kms_plane@plane-panning-bottom-right-suspend@pipe-a-planes.html
   [139]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/shard-

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/index.html

[-- Attachment #2: Type: text/html, Size: 33417 bytes --]

^ permalink raw reply	[flat|nested] 32+ messages in thread
* Re: [PATCH v5 1/4] drm/i915/guc: Add fetch of hwconfig table
  2022-02-22 10:36   ` [Intel-gfx] " Jordan Justen
@ 2022-02-24 13:58     ` Michal Wajdeczko
  -1 siblings, 0 replies; 32+ messages in thread
From: Michal Wajdeczko @ 2022-02-24 13:58 UTC (permalink / raw)
  To: Jordan Justen, intel-gfx
  Cc: Matthew Brost, Jon Bloomfield, John Harrison, dri-devel, Rodrigo Vivi



On 22.02.2022 11:36, Jordan Justen wrote:
> From: John Harrison <John.C.Harrison@Intel.com>
> 
> Implement support for fetching the hardware description table from the
> GuC. The call is made twice - once without a destination buffer to
> query the size and then a second time to fill in the buffer.
> 
> Note that the table is only available on ADL-P and later platforms.
> 
> v5 (of Jordan's posting):
>  * Various changes made by Jordan and recommended by Michal
>    - Makefile ordering
>    - Adjust "struct intel_guc_hwconfig hwconfig" comment
>    - Set Copyright year to 2022 in intel_guc_hwconfig.c/.h
>    - Drop inline from hwconfig_to_guc()
>    - Replace hwconfig param with guc in __guc_action_get_hwconfig()
>    - Move zero size check into guc_hwconfig_discover_size()
>    - Change comment to say zero size offset/size is needed to get size
>    - Add has_guc_hwconfig to devinfo and drop has_table()
>    - Change drm_err to notice in __uc_init_hw() and use %pe
> 
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
> Reviewed-by: Matthew Brost <matthew.brost@intel.com>
> Acked-by: Jon Bloomfield <jon.bloomfield@intel.com>
> Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
> ---
>  drivers/gpu/drm/i915/Makefile                 |   1 +
>  .../gpu/drm/i915/gt/uc/abi/guc_actions_abi.h  |   1 +
>  .../gpu/drm/i915/gt/uc/abi/guc_errors_abi.h   |   4 +
>  drivers/gpu/drm/i915/gt/uc/intel_guc.h        |   3 +
>  .../gpu/drm/i915/gt/uc/intel_guc_hwconfig.c   | 145 ++++++++++++++++++
>  .../gpu/drm/i915/gt/uc/intel_guc_hwconfig.h   |  19 +++
>  drivers/gpu/drm/i915/gt/uc/intel_uc.c         |   7 +
>  drivers/gpu/drm/i915/i915_pci.c               |   1 +
>  drivers/gpu/drm/i915/intel_device_info.h      |   1 +
>  9 files changed, 182 insertions(+)
>  create mode 100644 drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c
>  create mode 100644 drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.h
> 
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index e9ce09620eb5..661f1afb51d7 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -188,6 +188,7 @@ i915-y += gt/uc/intel_uc.o \
>  	  gt/uc/intel_guc_ct.o \
>  	  gt/uc/intel_guc_debugfs.o \
>  	  gt/uc/intel_guc_fw.o \
> +	  gt/uc/intel_guc_hwconfig.o \
>  	  gt/uc/intel_guc_log.o \
>  	  gt/uc/intel_guc_log_debugfs.o \
>  	  gt/uc/intel_guc_rc.o \
> diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h
> index fe5d7d261797..4a61c819f32b 100644
> --- a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h
> +++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h
> @@ -137,6 +137,7 @@ enum intel_guc_action {
>  	INTEL_GUC_ACTION_ENGINE_FAILURE_NOTIFICATION = 0x1009,
>  	INTEL_GUC_ACTION_SETUP_PC_GUCRC = 0x3004,
>  	INTEL_GUC_ACTION_AUTHENTICATE_HUC = 0x4000,
> +	INTEL_GUC_ACTION_GET_HWCONFIG = 0x4100,
>  	INTEL_GUC_ACTION_REGISTER_CONTEXT = 0x4502,
>  	INTEL_GUC_ACTION_DEREGISTER_CONTEXT = 0x4503,
>  	INTEL_GUC_ACTION_REGISTER_COMMAND_TRANSPORT_BUFFER = 0x4505,
> 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 488b6061ee89..f9e2a6aaef4a 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
> @@ -8,6 +8,10 @@
>  
>  enum intel_guc_response_status {
>  	INTEL_GUC_RESPONSE_STATUS_SUCCESS = 0x0,
> +	INTEL_GUC_RESPONSE_NOT_SUPPORTED = 0x20,
> +	INTEL_GUC_RESPONSE_NO_ATTRIBUTE_TABLE = 0x201,
> +	INTEL_GUC_RESPONSE_NO_DECRYPTION_KEY = 0x202,
> +	INTEL_GUC_RESPONSE_DECRYPTION_FAILED = 0x204,
>  	INTEL_GUC_RESPONSE_STATUS_GENERIC_FAIL = 0xF000,
>  };
>  
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> index f9240d4baa69..2058eb8c3d0c 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> @@ -13,6 +13,7 @@
>  #include "intel_guc_fw.h"
>  #include "intel_guc_fwif.h"
>  #include "intel_guc_ct.h"
> +#include "intel_guc_hwconfig.h"
>  #include "intel_guc_log.h"
>  #include "intel_guc_reg.h"
>  #include "intel_guc_slpc_types.h"
> @@ -37,6 +38,8 @@ struct intel_guc {
>  	struct intel_guc_ct ct;
>  	/** @slpc: sub-structure containing SLPC related data and objects */
>  	struct intel_guc_slpc slpc;
> +	/** @hwconfig: data related to hardware configuration KLV blob */
> +	struct intel_guc_hwconfig hwconfig;
>  
>  	/** @sched_engine: Global engine used to submit requests to GuC */
>  	struct i915_sched_engine *sched_engine;
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c
> new file mode 100644
> index 000000000000..ad289603460c
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c
> @@ -0,0 +1,145 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2022 Intel Corporation
> + */
> +
> +#include "gt/intel_gt.h"
> +#include "i915_drv.h"
> +#include "i915_memcpy.h"
> +#include "intel_guc_hwconfig.h"
> +
> +static struct intel_guc *hwconfig_to_guc(struct intel_guc_hwconfig *hwconfig)
> +{
> +	return container_of(hwconfig, struct intel_guc, hwconfig);
> +}
> +
> +/*
> + * GuC has a blob containing hardware configuration information (HWConfig).
> + * This is formatted as a simple and flexible KLV (Key/Length/Value) table.
> + *
> + * For example, a minimal version could be:
> + *   enum device_attr {
> + *     ATTR_SOME_VALUE = 0,
> + *     ATTR_SOME_MASK  = 1,
> + *   };
> + *
> + *   static const u32 hwconfig[] = {
> + *     ATTR_SOME_VALUE,
> + *     1,		// Value Length in DWords
> + *     8,		// Value
> + *
> + *     ATTR_SOME_MASK,
> + *     3,
> + *     0x00FFFFFFFF, 0xFFFFFFFF, 0xFF000000,
> + *   };
> + *
> + * The attribute ids are defined in a hardware spec.
> + */
> +
> +static int __guc_action_get_hwconfig(struct intel_guc *guc,
> +				     u32 ggtt_offset, u32 ggtt_size)
> +{
> +	u32 action[] = {
> +		INTEL_GUC_ACTION_GET_HWCONFIG,
> +		ggtt_offset,
> +		0, /* upper 32 bits of address */

nit: to avoid comments we can use

	lower_32_bits(ggtt_offset),
	upper_32_bits(ggtt_offset),

> +		ggtt_size,
> +	};
> +	int ret;
> +
> +	ret = intel_guc_send_mmio(guc, action, ARRAY_SIZE(action), NULL, 0);
> +	if (ret == -ENXIO)
> +		return -ENOENT;
> +
> +	return ret;
> +}
> +
> +static int guc_hwconfig_discover_size(struct intel_guc_hwconfig *hwconfig)
> +{
> +	struct intel_guc *guc = hwconfig_to_guc(hwconfig);
> +	int ret;
> +
> +	/* Sending a query with zero offset and size will return the
> +	 * size of the blob.
> +	 */

nit: wrong format of multi line comment

> +	ret = __guc_action_get_hwconfig(guc, 0, 0);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (ret == 0)
> +		return -EINVAL;
> +
> +	hwconfig->size = ret;
> +	return 0;
> +}
> +
> +static int guc_hwconfig_fill_buffer(struct intel_guc_hwconfig *hwconfig)
> +{
> +	struct intel_guc *guc = hwconfig_to_guc(hwconfig);
> +	struct i915_vma *vma;
> +	u32 ggtt_offset;
> +	void *vaddr;
> +	int ret;
> +
> +	GEM_BUG_ON(!hwconfig->size);
> +
> +	ret = intel_guc_allocate_and_map_vma(guc, hwconfig->size, &vma, &vaddr);
> +	if (ret)
> +		return ret;
> +
> +	ggtt_offset = intel_guc_ggtt_offset(guc, vma);
> +
> +	ret = __guc_action_get_hwconfig(guc, ggtt_offset, hwconfig->size);
> +	if (ret >= 0)
> +		memcpy(hwconfig->ptr, vaddr, hwconfig->size);
> +
> +	i915_vma_unpin_and_release(&vma, I915_VMA_RELEASE_MAP);
> +
> +	return ret;
> +}
> +
> +/**
> + * intel_guc_hwconfig_fini - Finalize the HWConfig
> + *
> + * Free up the memory allocation holding the table.
> + */
> +void intel_guc_hwconfig_fini(struct intel_guc_hwconfig *hwconfig)
> +{
> +	kfree(hwconfig->ptr);
> +	hwconfig->size = 0;
> +	hwconfig->ptr = NULL;
> +}
> +
> +/**
> + * intel_guc_hwconfig_init - Initialize the HWConfig
> + *
> + * Retrieve the HWConfig table from the GuC and save it away in a local memory

"local memory" may have different meaning, maybe ".. save it locally"

with comment reworded,

Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>

> + * allocation. It can then be queried on demand by other users later on.
> + */
> +int intel_guc_hwconfig_init(struct intel_guc_hwconfig *hwconfig)
> +{
> +	struct intel_guc *guc = hwconfig_to_guc(hwconfig);
> +	struct drm_i915_private *i915 = guc_to_gt(guc)->i915;
> +	int ret;
> +
> +	if (!INTEL_INFO(i915)->has_guc_hwconfig)
> +		return 0;
> +
> +	ret = guc_hwconfig_discover_size(hwconfig);
> +	if (ret)
> +		return ret;
> +
> +	hwconfig->ptr = kmalloc(hwconfig->size, GFP_KERNEL);
> +	if (!hwconfig->ptr) {
> +		hwconfig->size = 0;
> +		return -ENOMEM;
> +	}
> +
> +	ret = guc_hwconfig_fill_buffer(hwconfig);
> +	if (ret < 0) {
> +		intel_guc_hwconfig_fini(hwconfig);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.h
> new file mode 100644
> index 000000000000..bfb90ae168dc
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.h
> @@ -0,0 +1,19 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2022 Intel Corporation
> + */
> +
> +#ifndef _INTEL_GUC_HWCONFIG_H_
> +#define _INTEL_GUC_HWCONFIG_H_
> +
> +#include <linux/types.h>
> +
> +struct intel_guc_hwconfig {
> +	u32 size;
> +	void *ptr;
> +};
> +
> +int intel_guc_hwconfig_init(struct intel_guc_hwconfig *hwconfig);
> +void intel_guc_hwconfig_fini(struct intel_guc_hwconfig *hwconfig);
> +
> +#endif /* _INTEL_GUC_HWCONFIG_H_ */
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> index 09ed29df67bc..0cefa2a95190 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> @@ -489,6 +489,11 @@ static int __uc_init_hw(struct intel_uc *uc)
>  	if (ret)
>  		goto err_log_capture;
>  
> +	ret = intel_guc_hwconfig_init(&guc->hwconfig);
> +	if (ret)
> +		drm_notice(&i915->drm, "Failed to retrieve hwconfig table: %pe\n",
> +			   ERR_PTR(ret));
> +
>  	ret = guc_enable_communication(guc);
>  	if (ret)
>  		goto err_log_capture;
> @@ -562,6 +567,8 @@ static void __uc_fini_hw(struct intel_uc *uc)
>  	if (intel_uc_uses_guc_submission(uc))
>  		intel_guc_submission_disable(guc);
>  
> +	intel_guc_hwconfig_fini(&guc->hwconfig);
> +
>  	__uc_sanitize(uc);
>  }
>  
> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> index 76e590fcb903..1d31e35a5154 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -990,6 +990,7 @@ static const struct intel_device_info adl_p_info = {
>  		BIT(RCS0) | BIT(BCS0) | BIT(VECS0) | BIT(VCS0) | BIT(VCS2),
>  	.ppgtt_size = 48,
>  	.dma_mask_size = 39,
> +	.has_guc_hwconfig = 1,
>  };
>  
>  #undef GEN
> diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
> index 3699b1c539ea..82d8d6bc30ff 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.h
> +++ b/drivers/gpu/drm/i915/intel_device_info.h
> @@ -133,6 +133,7 @@ enum intel_ppgtt_type {
>  	func(gpu_reset_clobbers_display); \
>  	func(has_reset_engine); \
>  	func(has_global_mocs); \
> +	func(has_guc_hwconfig); \
>  	func(has_gt_uc); \
>  	func(has_l3_dpf); \
>  	func(has_llc); \

^ permalink raw reply	[flat|nested] 32+ messages in thread
* Re: [Intel-gfx] [PATCH v5 1/4] drm/i915/guc: Add fetch of hwconfig table
@ 2022-02-24 13:58     ` Michal Wajdeczko
  0 siblings, 0 replies; 32+ messages in thread
From: Michal Wajdeczko @ 2022-02-24 13:58 UTC (permalink / raw)
  To: Jordan Justen, intel-gfx; +Cc: dri-devel, Rodrigo Vivi



On 22.02.2022 11:36, Jordan Justen wrote:
> From: John Harrison <John.C.Harrison@Intel.com>
> 
> Implement support for fetching the hardware description table from the
> GuC. The call is made twice - once without a destination buffer to
> query the size and then a second time to fill in the buffer.
> 
> Note that the table is only available on ADL-P and later platforms.
> 
> v5 (of Jordan's posting):
>  * Various changes made by Jordan and recommended by Michal
>    - Makefile ordering
>    - Adjust "struct intel_guc_hwconfig hwconfig" comment
>    - Set Copyright year to 2022 in intel_guc_hwconfig.c/.h
>    - Drop inline from hwconfig_to_guc()
>    - Replace hwconfig param with guc in __guc_action_get_hwconfig()
>    - Move zero size check into guc_hwconfig_discover_size()
>    - Change comment to say zero size offset/size is needed to get size
>    - Add has_guc_hwconfig to devinfo and drop has_table()
>    - Change drm_err to notice in __uc_init_hw() and use %pe
> 
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
> Reviewed-by: Matthew Brost <matthew.brost@intel.com>
> Acked-by: Jon Bloomfield <jon.bloomfield@intel.com>
> Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
> ---
>  drivers/gpu/drm/i915/Makefile                 |   1 +
>  .../gpu/drm/i915/gt/uc/abi/guc_actions_abi.h  |   1 +
>  .../gpu/drm/i915/gt/uc/abi/guc_errors_abi.h   |   4 +
>  drivers/gpu/drm/i915/gt/uc/intel_guc.h        |   3 +
>  .../gpu/drm/i915/gt/uc/intel_guc_hwconfig.c   | 145 ++++++++++++++++++
>  .../gpu/drm/i915/gt/uc/intel_guc_hwconfig.h   |  19 +++
>  drivers/gpu/drm/i915/gt/uc/intel_uc.c         |   7 +
>  drivers/gpu/drm/i915/i915_pci.c               |   1 +
>  drivers/gpu/drm/i915/intel_device_info.h      |   1 +
>  9 files changed, 182 insertions(+)
>  create mode 100644 drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c
>  create mode 100644 drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.h
> 
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index e9ce09620eb5..661f1afb51d7 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -188,6 +188,7 @@ i915-y += gt/uc/intel_uc.o \
>  	  gt/uc/intel_guc_ct.o \
>  	  gt/uc/intel_guc_debugfs.o \
>  	  gt/uc/intel_guc_fw.o \
> +	  gt/uc/intel_guc_hwconfig.o \
>  	  gt/uc/intel_guc_log.o \
>  	  gt/uc/intel_guc_log_debugfs.o \
>  	  gt/uc/intel_guc_rc.o \
> diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h
> index fe5d7d261797..4a61c819f32b 100644
> --- a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h
> +++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h
> @@ -137,6 +137,7 @@ enum intel_guc_action {
>  	INTEL_GUC_ACTION_ENGINE_FAILURE_NOTIFICATION = 0x1009,
>  	INTEL_GUC_ACTION_SETUP_PC_GUCRC = 0x3004,
>  	INTEL_GUC_ACTION_AUTHENTICATE_HUC = 0x4000,
> +	INTEL_GUC_ACTION_GET_HWCONFIG = 0x4100,
>  	INTEL_GUC_ACTION_REGISTER_CONTEXT = 0x4502,
>  	INTEL_GUC_ACTION_DEREGISTER_CONTEXT = 0x4503,
>  	INTEL_GUC_ACTION_REGISTER_COMMAND_TRANSPORT_BUFFER = 0x4505,
> 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 488b6061ee89..f9e2a6aaef4a 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
> @@ -8,6 +8,10 @@
>  
>  enum intel_guc_response_status {
>  	INTEL_GUC_RESPONSE_STATUS_SUCCESS = 0x0,
> +	INTEL_GUC_RESPONSE_NOT_SUPPORTED = 0x20,
> +	INTEL_GUC_RESPONSE_NO_ATTRIBUTE_TABLE = 0x201,
> +	INTEL_GUC_RESPONSE_NO_DECRYPTION_KEY = 0x202,
> +	INTEL_GUC_RESPONSE_DECRYPTION_FAILED = 0x204,
>  	INTEL_GUC_RESPONSE_STATUS_GENERIC_FAIL = 0xF000,
>  };
>  
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> index f9240d4baa69..2058eb8c3d0c 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> @@ -13,6 +13,7 @@
>  #include "intel_guc_fw.h"
>  #include "intel_guc_fwif.h"
>  #include "intel_guc_ct.h"
> +#include "intel_guc_hwconfig.h"
>  #include "intel_guc_log.h"
>  #include "intel_guc_reg.h"
>  #include "intel_guc_slpc_types.h"
> @@ -37,6 +38,8 @@ struct intel_guc {
>  	struct intel_guc_ct ct;
>  	/** @slpc: sub-structure containing SLPC related data and objects */
>  	struct intel_guc_slpc slpc;
> +	/** @hwconfig: data related to hardware configuration KLV blob */
> +	struct intel_guc_hwconfig hwconfig;
>  
>  	/** @sched_engine: Global engine used to submit requests to GuC */
>  	struct i915_sched_engine *sched_engine;
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c
> new file mode 100644
> index 000000000000..ad289603460c
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c
> @@ -0,0 +1,145 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2022 Intel Corporation
> + */
> +
> +#include "gt/intel_gt.h"
> +#include "i915_drv.h"
> +#include "i915_memcpy.h"
> +#include "intel_guc_hwconfig.h"
> +
> +static struct intel_guc *hwconfig_to_guc(struct intel_guc_hwconfig *hwconfig)
> +{
> +	return container_of(hwconfig, struct intel_guc, hwconfig);
> +}
> +
> +/*
> + * GuC has a blob containing hardware configuration information (HWConfig).
> + * This is formatted as a simple and flexible KLV (Key/Length/Value) table.
> + *
> + * For example, a minimal version could be:
> + *   enum device_attr {
> + *     ATTR_SOME_VALUE = 0,
> + *     ATTR_SOME_MASK  = 1,
> + *   };
> + *
> + *   static const u32 hwconfig[] = {
> + *     ATTR_SOME_VALUE,
> + *     1,		// Value Length in DWords
> + *     8,		// Value
> + *
> + *     ATTR_SOME_MASK,
> + *     3,
> + *     0x00FFFFFFFF, 0xFFFFFFFF, 0xFF000000,
> + *   };
> + *
> + * The attribute ids are defined in a hardware spec.
> + */
> +
> +static int __guc_action_get_hwconfig(struct intel_guc *guc,
> +				     u32 ggtt_offset, u32 ggtt_size)
> +{
> +	u32 action[] = {
> +		INTEL_GUC_ACTION_GET_HWCONFIG,
> +		ggtt_offset,
> +		0, /* upper 32 bits of address */

nit: to avoid comments we can use

	lower_32_bits(ggtt_offset),
	upper_32_bits(ggtt_offset),

> +		ggtt_size,
> +	};
> +	int ret;
> +
> +	ret = intel_guc_send_mmio(guc, action, ARRAY_SIZE(action), NULL, 0);
> +	if (ret == -ENXIO)
> +		return -ENOENT;
> +
> +	return ret;
> +}
> +
> +static int guc_hwconfig_discover_size(struct intel_guc_hwconfig *hwconfig)
> +{
> +	struct intel_guc *guc = hwconfig_to_guc(hwconfig);
> +	int ret;
> +
> +	/* Sending a query with zero offset and size will return the
> +	 * size of the blob.
> +	 */

nit: wrong format of multi line comment

> +	ret = __guc_action_get_hwconfig(guc, 0, 0);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (ret == 0)
> +		return -EINVAL;
> +
> +	hwconfig->size = ret;
> +	return 0;
> +}
> +
> +static int guc_hwconfig_fill_buffer(struct intel_guc_hwconfig *hwconfig)
> +{
> +	struct intel_guc *guc = hwconfig_to_guc(hwconfig);
> +	struct i915_vma *vma;
> +	u32 ggtt_offset;
> +	void *vaddr;
> +	int ret;
> +
> +	GEM_BUG_ON(!hwconfig->size);
> +
> +	ret = intel_guc_allocate_and_map_vma(guc, hwconfig->size, &vma, &vaddr);
> +	if (ret)
> +		return ret;
> +
> +	ggtt_offset = intel_guc_ggtt_offset(guc, vma);
> +
> +	ret = __guc_action_get_hwconfig(guc, ggtt_offset, hwconfig->size);
> +	if (ret >= 0)
> +		memcpy(hwconfig->ptr, vaddr, hwconfig->size);
> +
> +	i915_vma_unpin_and_release(&vma, I915_VMA_RELEASE_MAP);
> +
> +	return ret;
> +}
> +
> +/**
> + * intel_guc_hwconfig_fini - Finalize the HWConfig
> + *
> + * Free up the memory allocation holding the table.
> + */
> +void intel_guc_hwconfig_fini(struct intel_guc_hwconfig *hwconfig)
> +{
> +	kfree(hwconfig->ptr);
> +	hwconfig->size = 0;
> +	hwconfig->ptr = NULL;
> +}
> +
> +/**
> + * intel_guc_hwconfig_init - Initialize the HWConfig
> + *
> + * Retrieve the HWConfig table from the GuC and save it away in a local memory

"local memory" may have different meaning, maybe ".. save it locally"

with comment reworded,

Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>

> + * allocation. It can then be queried on demand by other users later on.
> + */
> +int intel_guc_hwconfig_init(struct intel_guc_hwconfig *hwconfig)
> +{
> +	struct intel_guc *guc = hwconfig_to_guc(hwconfig);
> +	struct drm_i915_private *i915 = guc_to_gt(guc)->i915;
> +	int ret;
> +
> +	if (!INTEL_INFO(i915)->has_guc_hwconfig)
> +		return 0;
> +
> +	ret = guc_hwconfig_discover_size(hwconfig);
> +	if (ret)
> +		return ret;
> +
> +	hwconfig->ptr = kmalloc(hwconfig->size, GFP_KERNEL);
> +	if (!hwconfig->ptr) {
> +		hwconfig->size = 0;
> +		return -ENOMEM;
> +	}
> +
> +	ret = guc_hwconfig_fill_buffer(hwconfig);
> +	if (ret < 0) {
> +		intel_guc_hwconfig_fini(hwconfig);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.h
> new file mode 100644
> index 000000000000..bfb90ae168dc
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.h
> @@ -0,0 +1,19 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2022 Intel Corporation
> + */
> +
> +#ifndef _INTEL_GUC_HWCONFIG_H_
> +#define _INTEL_GUC_HWCONFIG_H_
> +
> +#include <linux/types.h>
> +
> +struct intel_guc_hwconfig {
> +	u32 size;
> +	void *ptr;
> +};
> +
> +int intel_guc_hwconfig_init(struct intel_guc_hwconfig *hwconfig);
> +void intel_guc_hwconfig_fini(struct intel_guc_hwconfig *hwconfig);
> +
> +#endif /* _INTEL_GUC_HWCONFIG_H_ */
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> index 09ed29df67bc..0cefa2a95190 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> @@ -489,6 +489,11 @@ static int __uc_init_hw(struct intel_uc *uc)
>  	if (ret)
>  		goto err_log_capture;
>  
> +	ret = intel_guc_hwconfig_init(&guc->hwconfig);
> +	if (ret)
> +		drm_notice(&i915->drm, "Failed to retrieve hwconfig table: %pe\n",
> +			   ERR_PTR(ret));
> +
>  	ret = guc_enable_communication(guc);
>  	if (ret)
>  		goto err_log_capture;
> @@ -562,6 +567,8 @@ static void __uc_fini_hw(struct intel_uc *uc)
>  	if (intel_uc_uses_guc_submission(uc))
>  		intel_guc_submission_disable(guc);
>  
> +	intel_guc_hwconfig_fini(&guc->hwconfig);
> +
>  	__uc_sanitize(uc);
>  }
>  
> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> index 76e590fcb903..1d31e35a5154 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -990,6 +990,7 @@ static const struct intel_device_info adl_p_info = {
>  		BIT(RCS0) | BIT(BCS0) | BIT(VECS0) | BIT(VCS0) | BIT(VCS2),
>  	.ppgtt_size = 48,
>  	.dma_mask_size = 39,
> +	.has_guc_hwconfig = 1,
>  };
>  
>  #undef GEN
> diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
> index 3699b1c539ea..82d8d6bc30ff 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.h
> +++ b/drivers/gpu/drm/i915/intel_device_info.h
> @@ -133,6 +133,7 @@ enum intel_ppgtt_type {
>  	func(gpu_reset_clobbers_display); \
>  	func(has_reset_engine); \
>  	func(has_global_mocs); \
> +	func(has_guc_hwconfig); \
>  	func(has_gt_uc); \
>  	func(has_l3_dpf); \
>  	func(has_llc); \

^ permalink raw reply	[flat|nested] 32+ messages in thread
* Re: [Intel-gfx]  ✓ Fi.CI.BAT: success for GuC HWCONFIG with documentation (rev5)
  2022-02-22 12:03 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
@ 2022-02-25  2:07   ` John Harrison
  0 siblings, 0 replies; 32+ messages in thread
From: John Harrison @ 2022-02-25  2:07 UTC (permalink / raw)
  To: intel-gfx, Patchwork, Jordan Justen

[-- Attachment #1: Type: text/plain, Size: 10380 bytes --]

On 2/22/2022 04:03, Patchwork wrote:
> Project List - Patchwork *Patch Details*
> *Series:* 	GuC HWCONFIG with documentation (rev5)
> *URL:* 	https://patchwork.freedesktop.org/series/99787/
> *State:* 	success
> *Details:* 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/index.html
>
>
>   CI Bug Log - changes from CI_DRM_11264 -> Patchwork_22348
>
>
>     Summary
>
> *SUCCESS*
>
> No regressions found.
>
> External URL: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/index.html
>
>
>     Participating hosts (44 -> 43)
>
> Additional (4): bat-dg2-8 fi-icl-u2 fi-bdw-5557u fi-pnv-d510
> Missing (5): fi-kbl-soraka shard-tglu fi-bsw-cyan shard-rkl shard-dg1
>
>
>     Possible new issues
>
> Here are the unknown changes that may have been introduced in 
> Patchwork_22348:
>
>
>       IGT changes
>
>
>         Suppressed
>
> The following results come from untrusted machines, tests, or statuses.
> They do not affect the overall result.
>
>  *
>
>     igt@i915_hangman@error-state-basic:
>
>       o {bat-adlp-6}: PASS
>         <https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11264/bat-adlp-6/igt@i915_hangman@error-state-basic.html>
>         -> DMESG-WARN
>         <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/bat-adlp-6/igt@i915_hangman@error-state-basic.html>
>
Despite the 'success' subject line, this is a failure caused by this 
patch set and it must be fixed before it can be merged.

John.

>  *
>  *
>
>     igt@i915_selftest@live:
>
>       o {fi-tgl-dsi}: NOTRUN -> FAIL
>         <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/fi-tgl-dsi/igt@i915_selftest@live.html>
>
>
>     Known issues
>
> Here are the changes found in Patchwork_22348 that come from known issues:
>
>
>       IGT changes
>
>
>         Issues hit
>
>  *
>
>     igt@amdgpu/amd_cs_nop@fork-gfx0:
>
>       o fi-icl-u2: NOTRUN -> SKIP
>         <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/fi-icl-u2/igt@amdgpu/amd_cs_nop@fork-gfx0.html>
>         (fdo#109315
>         <https://bugs.freedesktop.org/show_bug.cgi?id=109315>) +17
>         similar issues
>  *
>
>     igt@gem_exec_suspend@basic-s3@smem:
>
>       o fi-bdw-5557u: NOTRUN -> INCOMPLETE
>         <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/fi-bdw-5557u/igt@gem_exec_suspend@basic-s3@smem.html>
>         (i915#146 <https://gitlab.freedesktop.org/drm/intel/issues/146>)
>  *
>
>     igt@gem_huc_copy@huc-copy:
>
>      o
>
>         fi-skl-6600u: NOTRUN -> SKIP
>         <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/fi-skl-6600u/igt@gem_huc_copy@huc-copy.html>
>         (fdo#109271
>         <https://bugs.freedesktop.org/show_bug.cgi?id=109271> /
>         i915#2190 <https://gitlab.freedesktop.org/drm/intel/issues/2190>)
>
>      o
>
>         fi-pnv-d510: NOTRUN -> SKIP
>         <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/fi-pnv-d510/igt@gem_huc_copy@huc-copy.html>
>         (fdo#109271
>         <https://bugs.freedesktop.org/show_bug.cgi?id=109271>) +39
>         similar issues
>
>      o
>
>         fi-icl-u2: NOTRUN -> SKIP
>         <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/fi-icl-u2/igt@gem_huc_copy@huc-copy.html>
>         (i915#2190 <https://gitlab.freedesktop.org/drm/intel/issues/2190>)
>
>  *
>
>     igt@gem_lmem_swapping@parallel-random-engines:
>
>       o fi-icl-u2: NOTRUN -> SKIP
>         <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/fi-icl-u2/igt@gem_lmem_swapping@parallel-random-engines.html>
>         (i915#4613
>         <https://gitlab.freedesktop.org/drm/intel/issues/4613>) +3
>         similar issues
>  *
>
>     igt@gem_lmem_swapping@verify-random:
>
>       o fi-skl-6600u: NOTRUN -> SKIP
>         <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/fi-skl-6600u/igt@gem_lmem_swapping@verify-random.html>
>         (fdo#109271
>         <https://bugs.freedesktop.org/show_bug.cgi?id=109271> /
>         i915#4613
>         <https://gitlab.freedesktop.org/drm/intel/issues/4613>) +3
>         similar issues
>  *
>
>     igt@i915_selftest@live@requests:
>
>      o
>
>         fi-blb-e6850: PASS
>         <https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11264/fi-blb-e6850/igt@i915_selftest@live@requests.html>
>         -> DMESG-FAIL
>         <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/fi-blb-e6850/igt@i915_selftest@live@requests.html>
>         (i915#5026 <https://gitlab.freedesktop.org/drm/intel/issues/5026>)
>
>      o
>
>         fi-pnv-d510: NOTRUN -> DMESG-FAIL
>         <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/fi-pnv-d510/igt@i915_selftest@live@requests.html>
>         (i915#2927 <https://gitlab.freedesktop.org/drm/intel/issues/2927>)
>
>  *
>
>     igt@kms_chamelium@hdmi-hpd-fast:
>
>       o fi-icl-u2: NOTRUN -> SKIP
>         <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/fi-icl-u2/igt@kms_chamelium@hdmi-hpd-fast.html>
>         (fdo#111827
>         <https://bugs.freedesktop.org/show_bug.cgi?id=111827>) +8
>         similar issues
>  *
>
>     igt@kms_chamelium@vga-edid-read:
>
>       o fi-skl-6600u: NOTRUN -> SKIP
>         <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/fi-skl-6600u/igt@kms_chamelium@vga-edid-read.html>
>         (fdo#109271
>         <https://bugs.freedesktop.org/show_bug.cgi?id=109271> /
>         fdo#111827
>         <https://bugs.freedesktop.org/show_bug.cgi?id=111827>) +8
>         similar issues
>  *
>
>     igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
>
>       o fi-icl-u2: NOTRUN -> SKIP
>         <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/fi-icl-u2/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html>
>         (fdo#109278
>         <https://bugs.freedesktop.org/show_bug.cgi?id=109278>) +2
>         similar issues
>  *
>
>     igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy:
>
>       o fi-skl-6600u: NOTRUN -> SKIP
>         <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/fi-skl-6600u/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html>
>         (fdo#109271
>         <https://bugs.freedesktop.org/show_bug.cgi?id=109271>) +21
>         similar issues
>  *
>
>     igt@kms_force_connector_basic@force-load-detect:
>
>       o fi-icl-u2: NOTRUN -> SKIP
>         <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/fi-icl-u2/igt@kms_force_connector_basic@force-load-detect.html>
>         (fdo#109285 <https://bugs.freedesktop.org/show_bug.cgi?id=109285>)
>  *
>
>     igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d:
>
>       o fi-skl-6600u: NOTRUN -> SKIP
>         <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/fi-skl-6600u/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d.html>
>         (fdo#109271
>         <https://bugs.freedesktop.org/show_bug.cgi?id=109271> /
>         i915#533 <https://gitlab.freedesktop.org/drm/intel/issues/533>)
>  *
>
>     igt@prime_vgem@basic-userptr:
>
>       o fi-icl-u2: NOTRUN -> SKIP
>         <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/fi-icl-u2/igt@prime_vgem@basic-userptr.html>
>         (i915#3301 <https://gitlab.freedesktop.org/drm/intel/issues/3301>)
>  *
>
>     igt@runner@aborted:
>
>      o
>
>         fi-pnv-d510: NOTRUN -> FAIL
>         <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/fi-pnv-d510/igt@runner@aborted.html>
>         (fdo#109271
>         <https://bugs.freedesktop.org/show_bug.cgi?id=109271> /
>         i915#2403
>         <https://gitlab.freedesktop.org/drm/intel/issues/2403> /
>         i915#4312 <https://gitlab.freedesktop.org/drm/intel/issues/4312>)
>
>      o
>
>         fi-blb-e6850: NOTRUN -> FAIL
>         <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/fi-blb-e6850/igt@runner@aborted.html>
>         (fdo#109271
>         <https://bugs.freedesktop.org/show_bug.cgi?id=109271> /
>         i915#2403
>         <https://gitlab.freedesktop.org/drm/intel/issues/2403> /
>         i915#2426
>         <https://gitlab.freedesktop.org/drm/intel/issues/2426> /
>         i915#4312 <https://gitlab.freedesktop.org/drm/intel/issues/4312>)
>
>
>         Possible fixes
>
>  *
>
>     igt@gem_exec_suspend@basic-s3@smem:
>
>       o fi-skl-6600u: INCOMPLETE
>         <https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11264/fi-skl-6600u/igt@gem_exec_suspend@basic-s3@smem.html>
>         (i915#4547
>         <https://gitlab.freedesktop.org/drm/intel/issues/4547>) ->
>         PASS
>         <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/fi-skl-6600u/igt@gem_exec_suspend@basic-s3@smem.html>
>  *
>
>     igt@i915_selftest@live@gt_pm:
>
>       o fi-tgl-1115g4: DMESG-FAIL
>         <https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11264/fi-tgl-1115g4/igt@i915_selftest@live@gt_pm.html>
>         (i915#3987
>         <https://gitlab.freedesktop.org/drm/intel/issues/3987>) ->
>         PASS
>         <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/fi-tgl-1115g4/igt@i915_selftest@live@gt_pm.html>
>
>
>         Warnings
>
>   * igt@i915_selftest@live@hangcheck:
>       o fi-hsw-4770: INCOMPLETE
>         <https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11264/fi-hsw-4770/igt@i915_selftest@live@hangcheck.html>
>         (i915#3303
>         <https://gitlab.freedesktop.org/drm/intel/issues/3303>) ->
>         INCOMPLETE
>         <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22348/fi-hsw-4770/igt@i915_selftest@live@hangcheck.html>
>         (i915#4785 <https://gitlab.freedesktop.org/drm/intel/issues/4785>)
>
> {name}: This element is suppressed. This means it is ignored when 
> computing
> the status of the difference (SUCCESS, WARNING, or FAILURE).
>
>
>     Build changes
>
>   * Linux: CI_DRM_11264 -> Patchwork_22348
>
> CI-20190529: 20190529
> CI_DRM_11264: 3b562232559bd940e63f977619548e5cc70da985 @ 
> git://anongit.freedesktop.org/gfx-ci/linux
> IGT_6349: 0513032006f385f34fcd094c810b93f31cbed09d @ 
> https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
> Patchwork_22348: 5eee553b638ed3f343e129636be5fade999534e8 @ 
> git://anongit.freedesktop.org/gfx-ci/linux
>
> == Linux commits ==
>
> 5eee553b638e drm/i915/guc: Verify hwconfig blob matches supported format
> 2f033c0ee9f5 drm/i915/uapi: Add struct drm_i915_query_hwconfig_blob_item
> d5c4e6d3f58b drm/i915/uapi: Add query for hwconfig blob
> c3d64e01c80f drm/i915/guc: Add fetch of hwconfig table
>

[-- Attachment #2: Type: text/html, Size: 15215 bytes --]

^ permalink raw reply	[flat|nested] 32+ messages in thread
* Re: [PATCH v5 1/4] drm/i915/guc: Add fetch of hwconfig table
  2022-02-22 10:36   ` [Intel-gfx] " Jordan Justen
@ 2022-02-25  2:17     ` John Harrison
  -1 siblings, 0 replies; 32+ messages in thread
From: John Harrison @ 2022-02-25  2:17 UTC (permalink / raw)
  To: Jordan Justen, intel-gfx
  Cc: Matthew Brost, Rodrigo Vivi, Jon Bloomfield, dri-devel, Michal Wajdeczko

On 2/22/2022 02:36, Jordan Justen wrote:
> From: John Harrison <John.C.Harrison@Intel.com>
>
> Implement support for fetching the hardware description table from the
> GuC. The call is made twice - once without a destination buffer to
> query the size and then a second time to fill in the buffer.
>
> Note that the table is only available on ADL-P and later platforms.
>
> v5 (of Jordan's posting):
>   * Various changes made by Jordan and recommended by Michal
>     - Makefile ordering
>     - Adjust "struct intel_guc_hwconfig hwconfig" comment
>     - Set Copyright year to 2022 in intel_guc_hwconfig.c/.h
>     - Drop inline from hwconfig_to_guc()
>     - Replace hwconfig param with guc in __guc_action_get_hwconfig()
>     - Move zero size check into guc_hwconfig_discover_size()
>     - Change comment to say zero size offset/size is needed to get size
>     - Add has_guc_hwconfig to devinfo and drop has_table()
>     - Change drm_err to notice in __uc_init_hw() and use %pe
>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
> Reviewed-by: Matthew Brost <matthew.brost@intel.com>
> Acked-by: Jon Bloomfield <jon.bloomfield@intel.com>
> Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
> ---
>   drivers/gpu/drm/i915/Makefile                 |   1 +
>   .../gpu/drm/i915/gt/uc/abi/guc_actions_abi.h  |   1 +
>   .../gpu/drm/i915/gt/uc/abi/guc_errors_abi.h   |   4 +
>   drivers/gpu/drm/i915/gt/uc/intel_guc.h        |   3 +
>   .../gpu/drm/i915/gt/uc/intel_guc_hwconfig.c   | 145 ++++++++++++++++++
>   .../gpu/drm/i915/gt/uc/intel_guc_hwconfig.h   |  19 +++
>   drivers/gpu/drm/i915/gt/uc/intel_uc.c         |   7 +
>   drivers/gpu/drm/i915/i915_pci.c               |   1 +
>   drivers/gpu/drm/i915/intel_device_info.h      |   1 +
>   9 files changed, 182 insertions(+)
>   create mode 100644 drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c
>   create mode 100644 drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.h
>
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index e9ce09620eb5..661f1afb51d7 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -188,6 +188,7 @@ i915-y += gt/uc/intel_uc.o \
>   	  gt/uc/intel_guc_ct.o \
>   	  gt/uc/intel_guc_debugfs.o \
>   	  gt/uc/intel_guc_fw.o \
> +	  gt/uc/intel_guc_hwconfig.o \
>   	  gt/uc/intel_guc_log.o \
>   	  gt/uc/intel_guc_log_debugfs.o \
>   	  gt/uc/intel_guc_rc.o \
> diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h
> index fe5d7d261797..4a61c819f32b 100644
> --- a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h
> +++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h
> @@ -137,6 +137,7 @@ enum intel_guc_action {
>   	INTEL_GUC_ACTION_ENGINE_FAILURE_NOTIFICATION = 0x1009,
>   	INTEL_GUC_ACTION_SETUP_PC_GUCRC = 0x3004,
>   	INTEL_GUC_ACTION_AUTHENTICATE_HUC = 0x4000,
> +	INTEL_GUC_ACTION_GET_HWCONFIG = 0x4100,
>   	INTEL_GUC_ACTION_REGISTER_CONTEXT = 0x4502,
>   	INTEL_GUC_ACTION_DEREGISTER_CONTEXT = 0x4503,
>   	INTEL_GUC_ACTION_REGISTER_COMMAND_TRANSPORT_BUFFER = 0x4505,
> 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 488b6061ee89..f9e2a6aaef4a 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
> @@ -8,6 +8,10 @@
>   
>   enum intel_guc_response_status {
>   	INTEL_GUC_RESPONSE_STATUS_SUCCESS = 0x0,
> +	INTEL_GUC_RESPONSE_NOT_SUPPORTED = 0x20,
> +	INTEL_GUC_RESPONSE_NO_ATTRIBUTE_TABLE = 0x201,
> +	INTEL_GUC_RESPONSE_NO_DECRYPTION_KEY = 0x202,
> +	INTEL_GUC_RESPONSE_DECRYPTION_FAILED = 0x204,
>   	INTEL_GUC_RESPONSE_STATUS_GENERIC_FAIL = 0xF000,
>   };
>   
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> index f9240d4baa69..2058eb8c3d0c 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> @@ -13,6 +13,7 @@
>   #include "intel_guc_fw.h"
>   #include "intel_guc_fwif.h"
>   #include "intel_guc_ct.h"
> +#include "intel_guc_hwconfig.h"
>   #include "intel_guc_log.h"
>   #include "intel_guc_reg.h"
>   #include "intel_guc_slpc_types.h"
> @@ -37,6 +38,8 @@ struct intel_guc {
>   	struct intel_guc_ct ct;
>   	/** @slpc: sub-structure containing SLPC related data and objects */
>   	struct intel_guc_slpc slpc;
> +	/** @hwconfig: data related to hardware configuration KLV blob */
> +	struct intel_guc_hwconfig hwconfig;
>   
>   	/** @sched_engine: Global engine used to submit requests to GuC */
>   	struct i915_sched_engine *sched_engine;
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c
> new file mode 100644
> index 000000000000..ad289603460c
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c
> @@ -0,0 +1,145 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2022 Intel Corporation
> + */
> +
> +#include "gt/intel_gt.h"
> +#include "i915_drv.h"
> +#include "i915_memcpy.h"
> +#include "intel_guc_hwconfig.h"
> +
> +static struct intel_guc *hwconfig_to_guc(struct intel_guc_hwconfig *hwconfig)
> +{
> +	return container_of(hwconfig, struct intel_guc, hwconfig);
> +}
> +
> +/*
> + * GuC has a blob containing hardware configuration information (HWConfig).
> + * This is formatted as a simple and flexible KLV (Key/Length/Value) table.
> + *
> + * For example, a minimal version could be:
> + *   enum device_attr {
> + *     ATTR_SOME_VALUE = 0,
> + *     ATTR_SOME_MASK  = 1,
> + *   };
> + *
> + *   static const u32 hwconfig[] = {
> + *     ATTR_SOME_VALUE,
> + *     1,		// Value Length in DWords
> + *     8,		// Value
> + *
> + *     ATTR_SOME_MASK,
> + *     3,
> + *     0x00FFFFFFFF, 0xFFFFFFFF, 0xFF000000,
> + *   };
> + *
> + * The attribute ids are defined in a hardware spec.
> + */
> +
> +static int __guc_action_get_hwconfig(struct intel_guc *guc,
> +				     u32 ggtt_offset, u32 ggtt_size)
> +{
> +	u32 action[] = {
> +		INTEL_GUC_ACTION_GET_HWCONFIG,
> +		ggtt_offset,
> +		0, /* upper 32 bits of address */
> +		ggtt_size,
> +	};
> +	int ret;
> +
> +	ret = intel_guc_send_mmio(guc, action, ARRAY_SIZE(action), NULL, 0);
> +	if (ret == -ENXIO)
> +		return -ENOENT;
> +
> +	return ret;
> +}
> +
> +static int guc_hwconfig_discover_size(struct intel_guc_hwconfig *hwconfig)
> +{
> +	struct intel_guc *guc = hwconfig_to_guc(hwconfig);
> +	int ret;
> +
> +	/* Sending a query with zero offset and size will return the
> +	 * size of the blob.
> +	 */
> +	ret = __guc_action_get_hwconfig(guc, 0, 0);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (ret == 0)
> +		return -EINVAL;
> +
> +	hwconfig->size = ret;
> +	return 0;
> +}
> +
> +static int guc_hwconfig_fill_buffer(struct intel_guc_hwconfig *hwconfig)
> +{
> +	struct intel_guc *guc = hwconfig_to_guc(hwconfig);
> +	struct i915_vma *vma;
> +	u32 ggtt_offset;
> +	void *vaddr;
> +	int ret;
> +
> +	GEM_BUG_ON(!hwconfig->size);
> +
> +	ret = intel_guc_allocate_and_map_vma(guc, hwconfig->size, &vma, &vaddr);
> +	if (ret)
> +		return ret;
> +
> +	ggtt_offset = intel_guc_ggtt_offset(guc, vma);
> +
> +	ret = __guc_action_get_hwconfig(guc, ggtt_offset, hwconfig->size);
> +	if (ret >= 0)
> +		memcpy(hwconfig->ptr, vaddr, hwconfig->size);
> +
> +	i915_vma_unpin_and_release(&vma, I915_VMA_RELEASE_MAP);
> +
> +	return ret;
> +}
> +
> +/**
> + * intel_guc_hwconfig_fini - Finalize the HWConfig
> + *
> + * Free up the memory allocation holding the table.
> + */
> +void intel_guc_hwconfig_fini(struct intel_guc_hwconfig *hwconfig)
> +{
> +	kfree(hwconfig->ptr);
> +	hwconfig->size = 0;
> +	hwconfig->ptr = NULL;
> +}
> +
> +/**
> + * intel_guc_hwconfig_init - Initialize the HWConfig
> + *
> + * Retrieve the HWConfig table from the GuC and save it away in a local memory
> + * allocation. It can then be queried on demand by other users later on.
> + */
> +int intel_guc_hwconfig_init(struct intel_guc_hwconfig *hwconfig)
> +{
> +	struct intel_guc *guc = hwconfig_to_guc(hwconfig);
> +	struct drm_i915_private *i915 = guc_to_gt(guc)->i915;
> +	int ret;
> +
> +	if (!INTEL_INFO(i915)->has_guc_hwconfig)
> +		return 0;
> +
> +	ret = guc_hwconfig_discover_size(hwconfig);
> +	if (ret)
> +		return ret;
> +
> +	hwconfig->ptr = kmalloc(hwconfig->size, GFP_KERNEL);
> +	if (!hwconfig->ptr) {
> +		hwconfig->size = 0;
> +		return -ENOMEM;
> +	}
> +
> +	ret = guc_hwconfig_fill_buffer(hwconfig);
> +	if (ret < 0) {
> +		intel_guc_hwconfig_fini(hwconfig);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.h
> new file mode 100644
> index 000000000000..bfb90ae168dc
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.h
> @@ -0,0 +1,19 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2022 Intel Corporation
> + */
> +
> +#ifndef _INTEL_GUC_HWCONFIG_H_
> +#define _INTEL_GUC_HWCONFIG_H_
> +
> +#include <linux/types.h>
> +
> +struct intel_guc_hwconfig {
> +	u32 size;
> +	void *ptr;
> +};
> +
> +int intel_guc_hwconfig_init(struct intel_guc_hwconfig *hwconfig);
> +void intel_guc_hwconfig_fini(struct intel_guc_hwconfig *hwconfig);
> +
> +#endif /* _INTEL_GUC_HWCONFIG_H_ */
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> index 09ed29df67bc..0cefa2a95190 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> @@ -489,6 +489,11 @@ static int __uc_init_hw(struct intel_uc *uc)
>   	if (ret)
>   		goto err_log_capture;
>   
> +	ret = intel_guc_hwconfig_init(&guc->hwconfig);
> +	if (ret)
> +		drm_notice(&i915->drm, "Failed to retrieve hwconfig table: %pe\n",
Why only drm_notice? As you are keen to point out, the UMDs won't work 
if the table is not available. All the failure paths in your own 
verification function are 'drm_err'. So why is it only a 'notice' if 
there is no table at all?

Note that this function is called as part of the reset path. The reset 
path is not allowed to allocate memory. The table is stored in a 
dynamically allocated object. Hence the IGT test failure. The table 
query has to be done elsewhere at driver init time only.

> +			   ERR_PTR(ret));
> +
>   	ret = guc_enable_communication(guc);
>   	if (ret)
>   		goto err_log_capture;
> @@ -562,6 +567,8 @@ static void __uc_fini_hw(struct intel_uc *uc)
>   	if (intel_uc_uses_guc_submission(uc))
>   		intel_guc_submission_disable(guc);
>   
> +	intel_guc_hwconfig_fini(&guc->hwconfig);
> +
>   	__uc_sanitize(uc);
>   }
>   
> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> index 76e590fcb903..1d31e35a5154 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -990,6 +990,7 @@ static const struct intel_device_info adl_p_info = {
>   		BIT(RCS0) | BIT(BCS0) | BIT(VECS0) | BIT(VCS0) | BIT(VCS2),
>   	.ppgtt_size = 48,
>   	.dma_mask_size = 39,
> +	.has_guc_hwconfig = 1,
Who requested this change? It was previously done this way but the 
instruction was that i915_pci.c is for hardware features only but that 
this, as you seem extremely keen about pointing out at every 
opportunity, is a software feature.

John.


>   };
>   
>   #undef GEN
> diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
> index 3699b1c539ea..82d8d6bc30ff 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.h
> +++ b/drivers/gpu/drm/i915/intel_device_info.h
> @@ -133,6 +133,7 @@ enum intel_ppgtt_type {
>   	func(gpu_reset_clobbers_display); \
>   	func(has_reset_engine); \
>   	func(has_global_mocs); \
> +	func(has_guc_hwconfig); \
>   	func(has_gt_uc); \
>   	func(has_l3_dpf); \
>   	func(has_llc); \


^ permalink raw reply	[flat|nested] 32+ messages in thread
* Re: [Intel-gfx] [PATCH v5 1/4] drm/i915/guc: Add fetch of hwconfig table
@ 2022-02-25  2:17     ` John Harrison
  0 siblings, 0 replies; 32+ messages in thread
From: John Harrison @ 2022-02-25  2:17 UTC (permalink / raw)
  To: Jordan Justen, intel-gfx; +Cc: Rodrigo Vivi, dri-devel

On 2/22/2022 02:36, Jordan Justen wrote:
> From: John Harrison <John.C.Harrison@Intel.com>
>
> Implement support for fetching the hardware description table from the
> GuC. The call is made twice - once without a destination buffer to
> query the size and then a second time to fill in the buffer.
>
> Note that the table is only available on ADL-P and later platforms.
>
> v5 (of Jordan's posting):
>   * Various changes made by Jordan and recommended by Michal
>     - Makefile ordering
>     - Adjust "struct intel_guc_hwconfig hwconfig" comment
>     - Set Copyright year to 2022 in intel_guc_hwconfig.c/.h
>     - Drop inline from hwconfig_to_guc()
>     - Replace hwconfig param with guc in __guc_action_get_hwconfig()
>     - Move zero size check into guc_hwconfig_discover_size()
>     - Change comment to say zero size offset/size is needed to get size
>     - Add has_guc_hwconfig to devinfo and drop has_table()
>     - Change drm_err to notice in __uc_init_hw() and use %pe
>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
> Reviewed-by: Matthew Brost <matthew.brost@intel.com>
> Acked-by: Jon Bloomfield <jon.bloomfield@intel.com>
> Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
> ---
>   drivers/gpu/drm/i915/Makefile                 |   1 +
>   .../gpu/drm/i915/gt/uc/abi/guc_actions_abi.h  |   1 +
>   .../gpu/drm/i915/gt/uc/abi/guc_errors_abi.h   |   4 +
>   drivers/gpu/drm/i915/gt/uc/intel_guc.h        |   3 +
>   .../gpu/drm/i915/gt/uc/intel_guc_hwconfig.c   | 145 ++++++++++++++++++
>   .../gpu/drm/i915/gt/uc/intel_guc_hwconfig.h   |  19 +++
>   drivers/gpu/drm/i915/gt/uc/intel_uc.c         |   7 +
>   drivers/gpu/drm/i915/i915_pci.c               |   1 +
>   drivers/gpu/drm/i915/intel_device_info.h      |   1 +
>   9 files changed, 182 insertions(+)
>   create mode 100644 drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c
>   create mode 100644 drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.h
>
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index e9ce09620eb5..661f1afb51d7 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -188,6 +188,7 @@ i915-y += gt/uc/intel_uc.o \
>   	  gt/uc/intel_guc_ct.o \
>   	  gt/uc/intel_guc_debugfs.o \
>   	  gt/uc/intel_guc_fw.o \
> +	  gt/uc/intel_guc_hwconfig.o \
>   	  gt/uc/intel_guc_log.o \
>   	  gt/uc/intel_guc_log_debugfs.o \
>   	  gt/uc/intel_guc_rc.o \
> diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h
> index fe5d7d261797..4a61c819f32b 100644
> --- a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h
> +++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h
> @@ -137,6 +137,7 @@ enum intel_guc_action {
>   	INTEL_GUC_ACTION_ENGINE_FAILURE_NOTIFICATION = 0x1009,
>   	INTEL_GUC_ACTION_SETUP_PC_GUCRC = 0x3004,
>   	INTEL_GUC_ACTION_AUTHENTICATE_HUC = 0x4000,
> +	INTEL_GUC_ACTION_GET_HWCONFIG = 0x4100,
>   	INTEL_GUC_ACTION_REGISTER_CONTEXT = 0x4502,
>   	INTEL_GUC_ACTION_DEREGISTER_CONTEXT = 0x4503,
>   	INTEL_GUC_ACTION_REGISTER_COMMAND_TRANSPORT_BUFFER = 0x4505,
> 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 488b6061ee89..f9e2a6aaef4a 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
> @@ -8,6 +8,10 @@
>   
>   enum intel_guc_response_status {
>   	INTEL_GUC_RESPONSE_STATUS_SUCCESS = 0x0,
> +	INTEL_GUC_RESPONSE_NOT_SUPPORTED = 0x20,
> +	INTEL_GUC_RESPONSE_NO_ATTRIBUTE_TABLE = 0x201,
> +	INTEL_GUC_RESPONSE_NO_DECRYPTION_KEY = 0x202,
> +	INTEL_GUC_RESPONSE_DECRYPTION_FAILED = 0x204,
>   	INTEL_GUC_RESPONSE_STATUS_GENERIC_FAIL = 0xF000,
>   };
>   
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> index f9240d4baa69..2058eb8c3d0c 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> @@ -13,6 +13,7 @@
>   #include "intel_guc_fw.h"
>   #include "intel_guc_fwif.h"
>   #include "intel_guc_ct.h"
> +#include "intel_guc_hwconfig.h"
>   #include "intel_guc_log.h"
>   #include "intel_guc_reg.h"
>   #include "intel_guc_slpc_types.h"
> @@ -37,6 +38,8 @@ struct intel_guc {
>   	struct intel_guc_ct ct;
>   	/** @slpc: sub-structure containing SLPC related data and objects */
>   	struct intel_guc_slpc slpc;
> +	/** @hwconfig: data related to hardware configuration KLV blob */
> +	struct intel_guc_hwconfig hwconfig;
>   
>   	/** @sched_engine: Global engine used to submit requests to GuC */
>   	struct i915_sched_engine *sched_engine;
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c
> new file mode 100644
> index 000000000000..ad289603460c
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c
> @@ -0,0 +1,145 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2022 Intel Corporation
> + */
> +
> +#include "gt/intel_gt.h"
> +#include "i915_drv.h"
> +#include "i915_memcpy.h"
> +#include "intel_guc_hwconfig.h"
> +
> +static struct intel_guc *hwconfig_to_guc(struct intel_guc_hwconfig *hwconfig)
> +{
> +	return container_of(hwconfig, struct intel_guc, hwconfig);
> +}
> +
> +/*
> + * GuC has a blob containing hardware configuration information (HWConfig).
> + * This is formatted as a simple and flexible KLV (Key/Length/Value) table.
> + *
> + * For example, a minimal version could be:
> + *   enum device_attr {
> + *     ATTR_SOME_VALUE = 0,
> + *     ATTR_SOME_MASK  = 1,
> + *   };
> + *
> + *   static const u32 hwconfig[] = {
> + *     ATTR_SOME_VALUE,
> + *     1,		// Value Length in DWords
> + *     8,		// Value
> + *
> + *     ATTR_SOME_MASK,
> + *     3,
> + *     0x00FFFFFFFF, 0xFFFFFFFF, 0xFF000000,
> + *   };
> + *
> + * The attribute ids are defined in a hardware spec.
> + */
> +
> +static int __guc_action_get_hwconfig(struct intel_guc *guc,
> +				     u32 ggtt_offset, u32 ggtt_size)
> +{
> +	u32 action[] = {
> +		INTEL_GUC_ACTION_GET_HWCONFIG,
> +		ggtt_offset,
> +		0, /* upper 32 bits of address */
> +		ggtt_size,
> +	};
> +	int ret;
> +
> +	ret = intel_guc_send_mmio(guc, action, ARRAY_SIZE(action), NULL, 0);
> +	if (ret == -ENXIO)
> +		return -ENOENT;
> +
> +	return ret;
> +}
> +
> +static int guc_hwconfig_discover_size(struct intel_guc_hwconfig *hwconfig)
> +{
> +	struct intel_guc *guc = hwconfig_to_guc(hwconfig);
> +	int ret;
> +
> +	/* Sending a query with zero offset and size will return the
> +	 * size of the blob.
> +	 */
> +	ret = __guc_action_get_hwconfig(guc, 0, 0);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (ret == 0)
> +		return -EINVAL;
> +
> +	hwconfig->size = ret;
> +	return 0;
> +}
> +
> +static int guc_hwconfig_fill_buffer(struct intel_guc_hwconfig *hwconfig)
> +{
> +	struct intel_guc *guc = hwconfig_to_guc(hwconfig);
> +	struct i915_vma *vma;
> +	u32 ggtt_offset;
> +	void *vaddr;
> +	int ret;
> +
> +	GEM_BUG_ON(!hwconfig->size);
> +
> +	ret = intel_guc_allocate_and_map_vma(guc, hwconfig->size, &vma, &vaddr);
> +	if (ret)
> +		return ret;
> +
> +	ggtt_offset = intel_guc_ggtt_offset(guc, vma);
> +
> +	ret = __guc_action_get_hwconfig(guc, ggtt_offset, hwconfig->size);
> +	if (ret >= 0)
> +		memcpy(hwconfig->ptr, vaddr, hwconfig->size);
> +
> +	i915_vma_unpin_and_release(&vma, I915_VMA_RELEASE_MAP);
> +
> +	return ret;
> +}
> +
> +/**
> + * intel_guc_hwconfig_fini - Finalize the HWConfig
> + *
> + * Free up the memory allocation holding the table.
> + */
> +void intel_guc_hwconfig_fini(struct intel_guc_hwconfig *hwconfig)
> +{
> +	kfree(hwconfig->ptr);
> +	hwconfig->size = 0;
> +	hwconfig->ptr = NULL;
> +}
> +
> +/**
> + * intel_guc_hwconfig_init - Initialize the HWConfig
> + *
> + * Retrieve the HWConfig table from the GuC and save it away in a local memory
> + * allocation. It can then be queried on demand by other users later on.
> + */
> +int intel_guc_hwconfig_init(struct intel_guc_hwconfig *hwconfig)
> +{
> +	struct intel_guc *guc = hwconfig_to_guc(hwconfig);
> +	struct drm_i915_private *i915 = guc_to_gt(guc)->i915;
> +	int ret;
> +
> +	if (!INTEL_INFO(i915)->has_guc_hwconfig)
> +		return 0;
> +
> +	ret = guc_hwconfig_discover_size(hwconfig);
> +	if (ret)
> +		return ret;
> +
> +	hwconfig->ptr = kmalloc(hwconfig->size, GFP_KERNEL);
> +	if (!hwconfig->ptr) {
> +		hwconfig->size = 0;
> +		return -ENOMEM;
> +	}
> +
> +	ret = guc_hwconfig_fill_buffer(hwconfig);
> +	if (ret < 0) {
> +		intel_guc_hwconfig_fini(hwconfig);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.h
> new file mode 100644
> index 000000000000..bfb90ae168dc
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.h
> @@ -0,0 +1,19 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2022 Intel Corporation
> + */
> +
> +#ifndef _INTEL_GUC_HWCONFIG_H_
> +#define _INTEL_GUC_HWCONFIG_H_
> +
> +#include <linux/types.h>
> +
> +struct intel_guc_hwconfig {
> +	u32 size;
> +	void *ptr;
> +};
> +
> +int intel_guc_hwconfig_init(struct intel_guc_hwconfig *hwconfig);
> +void intel_guc_hwconfig_fini(struct intel_guc_hwconfig *hwconfig);
> +
> +#endif /* _INTEL_GUC_HWCONFIG_H_ */
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> index 09ed29df67bc..0cefa2a95190 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> @@ -489,6 +489,11 @@ static int __uc_init_hw(struct intel_uc *uc)
>   	if (ret)
>   		goto err_log_capture;
>   
> +	ret = intel_guc_hwconfig_init(&guc->hwconfig);
> +	if (ret)
> +		drm_notice(&i915->drm, "Failed to retrieve hwconfig table: %pe\n",
Why only drm_notice? As you are keen to point out, the UMDs won't work 
if the table is not available. All the failure paths in your own 
verification function are 'drm_err'. So why is it only a 'notice' if 
there is no table at all?

Note that this function is called as part of the reset path. The reset 
path is not allowed to allocate memory. The table is stored in a 
dynamically allocated object. Hence the IGT test failure. The table 
query has to be done elsewhere at driver init time only.

> +			   ERR_PTR(ret));
> +
>   	ret = guc_enable_communication(guc);
>   	if (ret)
>   		goto err_log_capture;
> @@ -562,6 +567,8 @@ static void __uc_fini_hw(struct intel_uc *uc)
>   	if (intel_uc_uses_guc_submission(uc))
>   		intel_guc_submission_disable(guc);
>   
> +	intel_guc_hwconfig_fini(&guc->hwconfig);
> +
>   	__uc_sanitize(uc);
>   }
>   
> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> index 76e590fcb903..1d31e35a5154 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -990,6 +990,7 @@ static const struct intel_device_info adl_p_info = {
>   		BIT(RCS0) | BIT(BCS0) | BIT(VECS0) | BIT(VCS0) | BIT(VCS2),
>   	.ppgtt_size = 48,
>   	.dma_mask_size = 39,
> +	.has_guc_hwconfig = 1,
Who requested this change? It was previously done this way but the 
instruction was that i915_pci.c is for hardware features only but that 
this, as you seem extremely keen about pointing out at every 
opportunity, is a software feature.

John.


>   };
>   
>   #undef GEN
> diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
> index 3699b1c539ea..82d8d6bc30ff 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.h
> +++ b/drivers/gpu/drm/i915/intel_device_info.h
> @@ -133,6 +133,7 @@ enum intel_ppgtt_type {
>   	func(gpu_reset_clobbers_display); \
>   	func(has_reset_engine); \
>   	func(has_global_mocs); \
> +	func(has_guc_hwconfig); \
>   	func(has_gt_uc); \
>   	func(has_l3_dpf); \
>   	func(has_llc); \


^ permalink raw reply	[flat|nested] 32+ messages in thread
* [PATCH] drm/i915/guc: Add fetch of hwconfig table
  2022-02-25  2:17     ` [Intel-gfx] " John Harrison
@ 2022-02-25  2:24       ` John.C.Harrison
  -1 siblings, 0 replies; 32+ messages in thread
From: John.C.Harrison @ 2022-02-25  2:24 UTC (permalink / raw)
  To: Intel-GFX
  Cc: Matthew Brost, Rodrigo Vivi, John Harrison, DRI-Devel, Michal Wajdeczko

From: John Harrison <John.C.Harrison@Intel.com>

Implement support for fetching the hardware description table from the
GuC. The call is made twice - once without a destination buffer to
query the size and then a second time to fill in the buffer.

The table is stored in the GT structure so that it can be fetched once
at driver load time. Keeping inside a GuC structure would mean it
would be release and reloaded on a GuC reset (part of a full GT
reset). However, the table does not change just because the GT has been
reset and the GuC reloaded. Also, dynamic memory allocations inside
the reset path are a problem.

Note that the table is only available on ADL-P and later platforms.

v2: Move to GT level to avoid memory allocation during reset path (and
unnecessary re-read of the table on a reset).

Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
Reviewed-by: Matthew Brost <matthew.brost@intel.com> (v1)

Mush: hwconf
---
 drivers/gpu/drm/i915/Makefile                 |   1 +
 drivers/gpu/drm/i915/gt/intel_gt.c            |   6 +
 drivers/gpu/drm/i915/gt/intel_gt_types.h      |   4 +
 drivers/gpu/drm/i915/gt/intel_hwconfig.h      |  21 +++
 .../gpu/drm/i915/gt/uc/abi/guc_actions_abi.h  |   1 +
 .../gpu/drm/i915/gt/uc/abi/guc_errors_abi.h   |   4 +
 .../gpu/drm/i915/gt/uc/intel_guc_hwconfig.c   | 160 ++++++++++++++++++
 7 files changed, 197 insertions(+)
 create mode 100644 drivers/gpu/drm/i915/gt/intel_hwconfig.h
 create mode 100644 drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 713f59a88312..8da4a1fe8b36 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -193,6 +193,7 @@ i915-y += gt/uc/intel_uc.o \
 	  gt/uc/intel_guc_rc.o \
 	  gt/uc/intel_guc_slpc.o \
 	  gt/uc/intel_guc_submission.o \
+	  gt/uc/intel_guc_hwconfig.o \
 	  gt/uc/intel_huc.o \
 	  gt/uc/intel_huc_debugfs.o \
 	  gt/uc/intel_huc_fw.o
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
index e6f6bf7c3926..2ad55d7ffe53 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt.c
@@ -711,6 +711,11 @@ int intel_gt_init(struct intel_gt *gt)
 	if (err)
 		goto err_uc_init;
 
+	/* Table failure is bad but not currently fatal */
+	err = intel_gt_init_hwconfig(gt);
+	if (err)
+		drm_err(&gt->i915->drm, "Failed to retrieve hwconfig table: %d\n", err);
+
 	err = __engines_record_defaults(gt);
 	if (err)
 		goto err_gt;
@@ -792,6 +797,7 @@ void intel_gt_driver_release(struct intel_gt *gt)
 	intel_gt_pm_fini(gt);
 	intel_gt_fini_scratch(gt);
 	intel_gt_fini_buffer_pool(gt);
+	intel_gt_fini_hwconfig(gt);
 }
 
 void intel_gt_driver_late_release(struct intel_gt *gt)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h
index f20687796490..514b92cff9b0 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h
@@ -20,6 +20,7 @@
 #include "i915_vma.h"
 #include "intel_engine_types.h"
 #include "intel_gt_buffer_pool_types.h"
+#include "intel_hwconfig.h"
 #include "intel_llc_types.h"
 #include "intel_reset_types.h"
 #include "intel_rc6_types.h"
@@ -199,6 +200,9 @@ struct intel_gt {
 		struct sseu_dev_info sseu;
 
 		unsigned long mslice_mask;
+
+		/** @hwconfig: hardware configuration data */
+		struct intel_hwconfig hwconfig;
 	} info;
 
 	struct {
diff --git a/drivers/gpu/drm/i915/gt/intel_hwconfig.h b/drivers/gpu/drm/i915/gt/intel_hwconfig.h
new file mode 100644
index 000000000000..322290780b67
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/intel_hwconfig.h
@@ -0,0 +1,21 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2022 Intel Corporation
+ */
+
+#ifndef _INTEL_HWCONFIG_H_
+#define _INTEL_HWCONFIG_H_
+
+#include <linux/types.h>
+
+struct intel_gt;
+
+struct intel_hwconfig {
+	u32 size;
+	void *ptr;
+};
+
+int intel_gt_init_hwconfig(struct intel_gt *gt);
+void intel_gt_fini_hwconfig(struct intel_gt *gt);
+
+#endif /* _INTEL_HWCONFIG_H_ */
diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h
index 7afdadc7656f..a9a329e53c35 100644
--- a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h
+++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h
@@ -129,6 +129,7 @@ enum intel_guc_action {
 	INTEL_GUC_ACTION_ENGINE_FAILURE_NOTIFICATION = 0x1009,
 	INTEL_GUC_ACTION_SETUP_PC_GUCRC = 0x3004,
 	INTEL_GUC_ACTION_AUTHENTICATE_HUC = 0x4000,
+	INTEL_GUC_ACTION_GET_HWCONFIG = 0x4100,
 	INTEL_GUC_ACTION_REGISTER_CONTEXT = 0x4502,
 	INTEL_GUC_ACTION_DEREGISTER_CONTEXT = 0x4503,
 	INTEL_GUC_ACTION_REGISTER_COMMAND_TRANSPORT_BUFFER = 0x4505,
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 c20658ee85a5..8085fb181274 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
@@ -8,6 +8,10 @@
 
 enum intel_guc_response_status {
 	INTEL_GUC_RESPONSE_STATUS_SUCCESS = 0x0,
+	INTEL_GUC_RESPONSE_NOT_SUPPORTED = 0x20,
+	INTEL_GUC_RESPONSE_NO_ATTRIBUTE_TABLE = 0x201,
+	INTEL_GUC_RESPONSE_NO_DECRYPTION_KEY = 0x202,
+	INTEL_GUC_RESPONSE_DECRYPTION_FAILED = 0x204,
 	INTEL_GUC_RESPONSE_STATUS_GENERIC_FAIL = 0xF000,
 };
 
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c
new file mode 100644
index 000000000000..d2dcd1e538ce
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c
@@ -0,0 +1,160 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2022 Intel Corporation
+ */
+
+#include "gt/intel_gt.h"
+#include "gt/intel_hwconfig.h"
+#include "i915_drv.h"
+#include "i915_memcpy.h"
+
+/*
+ * GuC has a blob containing hardware configuration information (HWConfig).
+ * This is formatted as a simple and flexible KLV (Key/Length/Value) table.
+ *
+ * For example, a minimal version could be:
+ *   enum device_attr {
+ *     ATTR_SOME_VALUE = 0,
+ *     ATTR_SOME_MASK  = 1,
+ *   };
+ *
+ *   static const u32 hwconfig[] = {
+ *     ATTR_SOME_VALUE,
+ *     1,		// Value Length in DWords
+ *     8,		// Value
+ *
+ *     ATTR_SOME_MASK,
+ *     3,
+ *     0x00FFFFFFFF, 0xFFFFFFFF, 0xFF000000,
+ *   };
+ *
+ * The attribute ids are defined in a hardware spec.
+ */
+
+static int __guc_action_get_hwconfig(struct intel_guc *guc,
+				     struct intel_hwconfig *hwconfig,
+				     u32 ggtt_offset, u32 ggtt_size)
+{
+	u32 action[] = {
+		INTEL_GUC_ACTION_GET_HWCONFIG,
+		ggtt_offset,
+		0, /* upper 32 bits of address */
+		ggtt_size,
+	};
+	int ret;
+
+	ret = intel_guc_send_mmio(guc, action, ARRAY_SIZE(action), NULL, 0);
+	if (ret == -ENXIO)
+		return -ENOENT;
+
+	if (!ggtt_size && !ret)
+		ret = -EINVAL;
+
+	return ret;
+}
+
+static int guc_hwconfig_discover_size(struct intel_guc *guc, struct intel_hwconfig *hwconfig)
+{
+	int ret;
+
+	/* Sending a query with zero offset/size will return the size of the table */
+	ret = __guc_action_get_hwconfig(guc, hwconfig, 0, 0);
+	if (ret < 0)
+		return ret;
+
+	hwconfig->size = ret;
+	return 0;
+}
+
+static int guc_hwconfig_fill_buffer(struct intel_guc *guc, struct intel_hwconfig *hwconfig)
+{
+	struct i915_vma *vma;
+	u32 ggtt_offset;
+	void *vaddr;
+	int ret;
+
+	GEM_BUG_ON(!hwconfig->size);
+
+	ret = intel_guc_allocate_and_map_vma(guc, hwconfig->size, &vma, &vaddr);
+	if (ret)
+		return ret;
+
+	ggtt_offset = intel_guc_ggtt_offset(guc, vma);
+
+	ret = __guc_action_get_hwconfig(guc, hwconfig, ggtt_offset, hwconfig->size);
+	if (ret >= 0)
+		memcpy(hwconfig->ptr, vaddr, hwconfig->size);
+
+	i915_vma_unpin_and_release(&vma, I915_VMA_RELEASE_MAP);
+
+	return ret;
+}
+
+static bool has_table(struct drm_i915_private *i915)
+{
+	if (IS_ALDERLAKE_P(i915))
+		return true;
+
+	return false;
+}
+
+/**
+ * intel_guc_hwconfig_init - Initialize the HWConfig
+ *
+ * Retrieve the HWConfig table from the GuC and save it away in a local memory
+ * allocation. It can then be queried on demand by other users later on.
+ */
+static int guc_hwconfig_init(struct intel_gt *gt)
+{
+	struct intel_hwconfig *hwconfig = &gt->info.hwconfig;
+	struct intel_guc *guc = &gt->uc.guc;
+	int ret;
+
+	if (!has_table(gt->i915))
+		return 0;
+
+	ret = guc_hwconfig_discover_size(guc, hwconfig);
+	if (ret)
+		return ret;
+
+	hwconfig->ptr = kmalloc(hwconfig->size, GFP_KERNEL);
+	if (!hwconfig->ptr) {
+		hwconfig->size = 0;
+		return -ENOMEM;
+	}
+
+	ret = guc_hwconfig_fill_buffer(guc, hwconfig);
+	if (ret < 0) {
+		intel_gt_fini_hwconfig(gt);
+		return ret;
+	}
+
+	return 0;
+}
+
+/**
+ * intel_gt_init_hwconfig - Initialize the HWConfig if available
+ *
+ * Retrieve the HWConfig table if available on the current platform.
+ */
+int intel_gt_init_hwconfig(struct intel_gt *gt)
+{
+	if (!intel_uc_uses_guc(&gt->uc))
+		return 0;
+
+	return guc_hwconfig_init(gt);
+}
+
+/**
+ * intel_gt_fini_hwconfig - Finalize the HWConfig
+ *
+ * Free up the memory allocation holding the table.
+ */
+void intel_gt_fini_hwconfig(struct intel_gt *gt)
+{
+	struct intel_hwconfig *hwconfig = &gt->info.hwconfig;
+
+	kfree(hwconfig->ptr);
+	hwconfig->size = 0;
+	hwconfig->ptr = NULL;
+}
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread
* [Intel-gfx] [PATCH] drm/i915/guc: Add fetch of hwconfig table
@ 2022-02-25  2:24       ` John.C.Harrison
  0 siblings, 0 replies; 32+ messages in thread
From: John.C.Harrison @ 2022-02-25  2:24 UTC (permalink / raw)
  To: Intel-GFX; +Cc: Rodrigo Vivi, DRI-Devel

From: John Harrison <John.C.Harrison@Intel.com>

Implement support for fetching the hardware description table from the
GuC. The call is made twice - once without a destination buffer to
query the size and then a second time to fill in the buffer.

The table is stored in the GT structure so that it can be fetched once
at driver load time. Keeping inside a GuC structure would mean it
would be release and reloaded on a GuC reset (part of a full GT
reset). However, the table does not change just because the GT has been
reset and the GuC reloaded. Also, dynamic memory allocations inside
the reset path are a problem.

Note that the table is only available on ADL-P and later platforms.

v2: Move to GT level to avoid memory allocation during reset path (and
unnecessary re-read of the table on a reset).

Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
Reviewed-by: Matthew Brost <matthew.brost@intel.com> (v1)

Mush: hwconf
---
 drivers/gpu/drm/i915/Makefile                 |   1 +
 drivers/gpu/drm/i915/gt/intel_gt.c            |   6 +
 drivers/gpu/drm/i915/gt/intel_gt_types.h      |   4 +
 drivers/gpu/drm/i915/gt/intel_hwconfig.h      |  21 +++
 .../gpu/drm/i915/gt/uc/abi/guc_actions_abi.h  |   1 +
 .../gpu/drm/i915/gt/uc/abi/guc_errors_abi.h   |   4 +
 .../gpu/drm/i915/gt/uc/intel_guc_hwconfig.c   | 160 ++++++++++++++++++
 7 files changed, 197 insertions(+)
 create mode 100644 drivers/gpu/drm/i915/gt/intel_hwconfig.h
 create mode 100644 drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 713f59a88312..8da4a1fe8b36 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -193,6 +193,7 @@ i915-y += gt/uc/intel_uc.o \
 	  gt/uc/intel_guc_rc.o \
 	  gt/uc/intel_guc_slpc.o \
 	  gt/uc/intel_guc_submission.o \
+	  gt/uc/intel_guc_hwconfig.o \
 	  gt/uc/intel_huc.o \
 	  gt/uc/intel_huc_debugfs.o \
 	  gt/uc/intel_huc_fw.o
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
index e6f6bf7c3926..2ad55d7ffe53 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt.c
@@ -711,6 +711,11 @@ int intel_gt_init(struct intel_gt *gt)
 	if (err)
 		goto err_uc_init;
 
+	/* Table failure is bad but not currently fatal */
+	err = intel_gt_init_hwconfig(gt);
+	if (err)
+		drm_err(&gt->i915->drm, "Failed to retrieve hwconfig table: %d\n", err);
+
 	err = __engines_record_defaults(gt);
 	if (err)
 		goto err_gt;
@@ -792,6 +797,7 @@ void intel_gt_driver_release(struct intel_gt *gt)
 	intel_gt_pm_fini(gt);
 	intel_gt_fini_scratch(gt);
 	intel_gt_fini_buffer_pool(gt);
+	intel_gt_fini_hwconfig(gt);
 }
 
 void intel_gt_driver_late_release(struct intel_gt *gt)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h
index f20687796490..514b92cff9b0 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h
@@ -20,6 +20,7 @@
 #include "i915_vma.h"
 #include "intel_engine_types.h"
 #include "intel_gt_buffer_pool_types.h"
+#include "intel_hwconfig.h"
 #include "intel_llc_types.h"
 #include "intel_reset_types.h"
 #include "intel_rc6_types.h"
@@ -199,6 +200,9 @@ struct intel_gt {
 		struct sseu_dev_info sseu;
 
 		unsigned long mslice_mask;
+
+		/** @hwconfig: hardware configuration data */
+		struct intel_hwconfig hwconfig;
 	} info;
 
 	struct {
diff --git a/drivers/gpu/drm/i915/gt/intel_hwconfig.h b/drivers/gpu/drm/i915/gt/intel_hwconfig.h
new file mode 100644
index 000000000000..322290780b67
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/intel_hwconfig.h
@@ -0,0 +1,21 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2022 Intel Corporation
+ */
+
+#ifndef _INTEL_HWCONFIG_H_
+#define _INTEL_HWCONFIG_H_
+
+#include <linux/types.h>
+
+struct intel_gt;
+
+struct intel_hwconfig {
+	u32 size;
+	void *ptr;
+};
+
+int intel_gt_init_hwconfig(struct intel_gt *gt);
+void intel_gt_fini_hwconfig(struct intel_gt *gt);
+
+#endif /* _INTEL_HWCONFIG_H_ */
diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h
index 7afdadc7656f..a9a329e53c35 100644
--- a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h
+++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h
@@ -129,6 +129,7 @@ enum intel_guc_action {
 	INTEL_GUC_ACTION_ENGINE_FAILURE_NOTIFICATION = 0x1009,
 	INTEL_GUC_ACTION_SETUP_PC_GUCRC = 0x3004,
 	INTEL_GUC_ACTION_AUTHENTICATE_HUC = 0x4000,
+	INTEL_GUC_ACTION_GET_HWCONFIG = 0x4100,
 	INTEL_GUC_ACTION_REGISTER_CONTEXT = 0x4502,
 	INTEL_GUC_ACTION_DEREGISTER_CONTEXT = 0x4503,
 	INTEL_GUC_ACTION_REGISTER_COMMAND_TRANSPORT_BUFFER = 0x4505,
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 c20658ee85a5..8085fb181274 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
@@ -8,6 +8,10 @@
 
 enum intel_guc_response_status {
 	INTEL_GUC_RESPONSE_STATUS_SUCCESS = 0x0,
+	INTEL_GUC_RESPONSE_NOT_SUPPORTED = 0x20,
+	INTEL_GUC_RESPONSE_NO_ATTRIBUTE_TABLE = 0x201,
+	INTEL_GUC_RESPONSE_NO_DECRYPTION_KEY = 0x202,
+	INTEL_GUC_RESPONSE_DECRYPTION_FAILED = 0x204,
 	INTEL_GUC_RESPONSE_STATUS_GENERIC_FAIL = 0xF000,
 };
 
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c
new file mode 100644
index 000000000000..d2dcd1e538ce
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c
@@ -0,0 +1,160 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2022 Intel Corporation
+ */
+
+#include "gt/intel_gt.h"
+#include "gt/intel_hwconfig.h"
+#include "i915_drv.h"
+#include "i915_memcpy.h"
+
+/*
+ * GuC has a blob containing hardware configuration information (HWConfig).
+ * This is formatted as a simple and flexible KLV (Key/Length/Value) table.
+ *
+ * For example, a minimal version could be:
+ *   enum device_attr {
+ *     ATTR_SOME_VALUE = 0,
+ *     ATTR_SOME_MASK  = 1,
+ *   };
+ *
+ *   static const u32 hwconfig[] = {
+ *     ATTR_SOME_VALUE,
+ *     1,		// Value Length in DWords
+ *     8,		// Value
+ *
+ *     ATTR_SOME_MASK,
+ *     3,
+ *     0x00FFFFFFFF, 0xFFFFFFFF, 0xFF000000,
+ *   };
+ *
+ * The attribute ids are defined in a hardware spec.
+ */
+
+static int __guc_action_get_hwconfig(struct intel_guc *guc,
+				     struct intel_hwconfig *hwconfig,
+				     u32 ggtt_offset, u32 ggtt_size)
+{
+	u32 action[] = {
+		INTEL_GUC_ACTION_GET_HWCONFIG,
+		ggtt_offset,
+		0, /* upper 32 bits of address */
+		ggtt_size,
+	};
+	int ret;
+
+	ret = intel_guc_send_mmio(guc, action, ARRAY_SIZE(action), NULL, 0);
+	if (ret == -ENXIO)
+		return -ENOENT;
+
+	if (!ggtt_size && !ret)
+		ret = -EINVAL;
+
+	return ret;
+}
+
+static int guc_hwconfig_discover_size(struct intel_guc *guc, struct intel_hwconfig *hwconfig)
+{
+	int ret;
+
+	/* Sending a query with zero offset/size will return the size of the table */
+	ret = __guc_action_get_hwconfig(guc, hwconfig, 0, 0);
+	if (ret < 0)
+		return ret;
+
+	hwconfig->size = ret;
+	return 0;
+}
+
+static int guc_hwconfig_fill_buffer(struct intel_guc *guc, struct intel_hwconfig *hwconfig)
+{
+	struct i915_vma *vma;
+	u32 ggtt_offset;
+	void *vaddr;
+	int ret;
+
+	GEM_BUG_ON(!hwconfig->size);
+
+	ret = intel_guc_allocate_and_map_vma(guc, hwconfig->size, &vma, &vaddr);
+	if (ret)
+		return ret;
+
+	ggtt_offset = intel_guc_ggtt_offset(guc, vma);
+
+	ret = __guc_action_get_hwconfig(guc, hwconfig, ggtt_offset, hwconfig->size);
+	if (ret >= 0)
+		memcpy(hwconfig->ptr, vaddr, hwconfig->size);
+
+	i915_vma_unpin_and_release(&vma, I915_VMA_RELEASE_MAP);
+
+	return ret;
+}
+
+static bool has_table(struct drm_i915_private *i915)
+{
+	if (IS_ALDERLAKE_P(i915))
+		return true;
+
+	return false;
+}
+
+/**
+ * intel_guc_hwconfig_init - Initialize the HWConfig
+ *
+ * Retrieve the HWConfig table from the GuC and save it away in a local memory
+ * allocation. It can then be queried on demand by other users later on.
+ */
+static int guc_hwconfig_init(struct intel_gt *gt)
+{
+	struct intel_hwconfig *hwconfig = &gt->info.hwconfig;
+	struct intel_guc *guc = &gt->uc.guc;
+	int ret;
+
+	if (!has_table(gt->i915))
+		return 0;
+
+	ret = guc_hwconfig_discover_size(guc, hwconfig);
+	if (ret)
+		return ret;
+
+	hwconfig->ptr = kmalloc(hwconfig->size, GFP_KERNEL);
+	if (!hwconfig->ptr) {
+		hwconfig->size = 0;
+		return -ENOMEM;
+	}
+
+	ret = guc_hwconfig_fill_buffer(guc, hwconfig);
+	if (ret < 0) {
+		intel_gt_fini_hwconfig(gt);
+		return ret;
+	}
+
+	return 0;
+}
+
+/**
+ * intel_gt_init_hwconfig - Initialize the HWConfig if available
+ *
+ * Retrieve the HWConfig table if available on the current platform.
+ */
+int intel_gt_init_hwconfig(struct intel_gt *gt)
+{
+	if (!intel_uc_uses_guc(&gt->uc))
+		return 0;
+
+	return guc_hwconfig_init(gt);
+}
+
+/**
+ * intel_gt_fini_hwconfig - Finalize the HWConfig
+ *
+ * Free up the memory allocation holding the table.
+ */
+void intel_gt_fini_hwconfig(struct intel_gt *gt)
+{
+	struct intel_hwconfig *hwconfig = &gt->info.hwconfig;
+
+	kfree(hwconfig->ptr);
+	hwconfig->size = 0;
+	hwconfig->ptr = NULL;
+}
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread
* Re: [PATCH v5 1/4] drm/i915/guc: Add fetch of hwconfig table
  2022-02-25  2:17     ` [Intel-gfx] " John Harrison
@ 2022-02-25  5:03       ` Jordan Justen
  -1 siblings, 0 replies; 32+ messages in thread
From: Jordan Justen @ 2022-02-25  5:03 UTC (permalink / raw)
  To: John Harrison, Michal Wajdeczko, intel-gfx
  Cc: Matthew Brost, Jon Bloomfield, dri-devel, Rodrigo Vivi

John Harrison <john.c.harrison@intel.com> writes:

> On 2/22/2022 02:36, Jordan Justen wrote:
>> From: John Harrison <John.C.Harrison@Intel.com>
>>
>> Implement support for fetching the hardware description table from the
>> GuC. The call is made twice - once without a destination buffer to
>> query the size and then a second time to fill in the buffer.
>>
>> Note that the table is only available on ADL-P and later platforms.
>>
>> v5 (of Jordan's posting):
>>   * Various changes made by Jordan and recommended by Michal
>>     - Makefile ordering
>>     - Adjust "struct intel_guc_hwconfig hwconfig" comment
>>     - Set Copyright year to 2022 in intel_guc_hwconfig.c/.h
>>     - Drop inline from hwconfig_to_guc()
>>     - Replace hwconfig param with guc in __guc_action_get_hwconfig()
>>     - Move zero size check into guc_hwconfig_discover_size()
>>     - Change comment to say zero size offset/size is needed to get size
>>     - Add has_guc_hwconfig to devinfo and drop has_table()
>>     - Change drm_err to notice in __uc_init_hw() and use %pe
>>
>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
>> Reviewed-by: Matthew Brost <matthew.brost@intel.com>
>> Acked-by: Jon Bloomfield <jon.bloomfield@intel.com>
>> Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
>> ---
>>   
>> +	ret = intel_guc_hwconfig_init(&guc->hwconfig);
>> +	if (ret)
>> +		drm_notice(&i915->drm, "Failed to retrieve hwconfig table: %pe\n",
> Why only drm_notice? As you are keen to point out, the UMDs won't work 
> if the table is not available. All the failure paths in your own 
> verification function are 'drm_err'. So why is it only a 'notice' if 
> there is no table at all?

This was requested by Michal in my v3 posting:

https://patchwork.freedesktop.org/patch/472936/?series=99787&rev=3

I don't think that it should be a failure for i915 if it is unable to
read the table, or if the table read is invalid. I think it should be up
to the UMD to react to the missing hwconfig however they think is
appropriate, but I would like the i915 to guarantee & document the
format returned to userspace to whatever extent is feasible.

As you point out there is a discrepancy, and I think I should be
consistent with whatever is used here in my "drm/i915/guc: Verify
hwconfig blob matches supported format" patch.

I guess I'd tend to agree with Michal that "maybe drm_notice since we
continue probe", but I would go along with either if you two want to
discuss further.

> Note that this function is called as part of the reset path. The reset 
> path is not allowed to allocate memory. The table is stored in a 
> dynamically allocated object. Hence the IGT test failure. The table 
> query has to be done elsewhere at driver init time only.

Thanks for clearing this up. I did notice on dg2 that gpu resets were
causing a re-read of the hwconfig from GuC, but it definitely was not
clear to me that there would be a connection to the IGT failure that you
pointed out.

>
>> +			   ERR_PTR(ret));
>> +
>>   	ret = guc_enable_communication(guc);
>>   	if (ret)
>>   		goto err_log_capture;
>> @@ -562,6 +567,8 @@ static void __uc_fini_hw(struct intel_uc *uc)
>>   	if (intel_uc_uses_guc_submission(uc))
>>   		intel_guc_submission_disable(guc);
>>   
>> +	intel_guc_hwconfig_fini(&guc->hwconfig);
>> +
>>   	__uc_sanitize(uc);
>>   }
>>   
>> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
>> index 76e590fcb903..1d31e35a5154 100644
>> --- a/drivers/gpu/drm/i915/i915_pci.c
>> +++ b/drivers/gpu/drm/i915/i915_pci.c
>> @@ -990,6 +990,7 @@ static const struct intel_device_info adl_p_info = {
>>   		BIT(RCS0) | BIT(BCS0) | BIT(VECS0) | BIT(VCS0) | BIT(VCS2),
>>   	.ppgtt_size = 48,
>>   	.dma_mask_size = 39,
>> +	.has_guc_hwconfig = 1,
> Who requested this change? It was previously done this way but the 
> instruction was that i915_pci.c is for hardware features only but that 
> this, as you seem extremely keen about pointing out at every 
> opportunity, is a software feature.

This was requested by Michal as well. I definitely agree it is a
software feature, but I was not aware that "i915_pci.c is for hardware
features only".

Michal, do you agree with this and returning to the previous method for
enabling the feature?

-Jordan

^ permalink raw reply	[flat|nested] 32+ messages in thread
* Re: [Intel-gfx] [PATCH v5 1/4] drm/i915/guc: Add fetch of hwconfig table
@ 2022-02-25  5:03       ` Jordan Justen
  0 siblings, 0 replies; 32+ messages in thread
From: Jordan Justen @ 2022-02-25  5:03 UTC (permalink / raw)
  To: John Harrison, Michal Wajdeczko, intel-gfx; +Cc: dri-devel, Rodrigo Vivi

John Harrison <john.c.harrison@intel.com> writes:

> On 2/22/2022 02:36, Jordan Justen wrote:
>> From: John Harrison <John.C.Harrison@Intel.com>
>>
>> Implement support for fetching the hardware description table from the
>> GuC. The call is made twice - once without a destination buffer to
>> query the size and then a second time to fill in the buffer.
>>
>> Note that the table is only available on ADL-P and later platforms.
>>
>> v5 (of Jordan's posting):
>>   * Various changes made by Jordan and recommended by Michal
>>     - Makefile ordering
>>     - Adjust "struct intel_guc_hwconfig hwconfig" comment
>>     - Set Copyright year to 2022 in intel_guc_hwconfig.c/.h
>>     - Drop inline from hwconfig_to_guc()
>>     - Replace hwconfig param with guc in __guc_action_get_hwconfig()
>>     - Move zero size check into guc_hwconfig_discover_size()
>>     - Change comment to say zero size offset/size is needed to get size
>>     - Add has_guc_hwconfig to devinfo and drop has_table()
>>     - Change drm_err to notice in __uc_init_hw() and use %pe
>>
>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
>> Reviewed-by: Matthew Brost <matthew.brost@intel.com>
>> Acked-by: Jon Bloomfield <jon.bloomfield@intel.com>
>> Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
>> ---
>>   
>> +	ret = intel_guc_hwconfig_init(&guc->hwconfig);
>> +	if (ret)
>> +		drm_notice(&i915->drm, "Failed to retrieve hwconfig table: %pe\n",
> Why only drm_notice? As you are keen to point out, the UMDs won't work 
> if the table is not available. All the failure paths in your own 
> verification function are 'drm_err'. So why is it only a 'notice' if 
> there is no table at all?

This was requested by Michal in my v3 posting:

https://patchwork.freedesktop.org/patch/472936/?series=99787&rev=3

I don't think that it should be a failure for i915 if it is unable to
read the table, or if the table read is invalid. I think it should be up
to the UMD to react to the missing hwconfig however they think is
appropriate, but I would like the i915 to guarantee & document the
format returned to userspace to whatever extent is feasible.

As you point out there is a discrepancy, and I think I should be
consistent with whatever is used here in my "drm/i915/guc: Verify
hwconfig blob matches supported format" patch.

I guess I'd tend to agree with Michal that "maybe drm_notice since we
continue probe", but I would go along with either if you two want to
discuss further.

> Note that this function is called as part of the reset path. The reset 
> path is not allowed to allocate memory. The table is stored in a 
> dynamically allocated object. Hence the IGT test failure. The table 
> query has to be done elsewhere at driver init time only.

Thanks for clearing this up. I did notice on dg2 that gpu resets were
causing a re-read of the hwconfig from GuC, but it definitely was not
clear to me that there would be a connection to the IGT failure that you
pointed out.

>
>> +			   ERR_PTR(ret));
>> +
>>   	ret = guc_enable_communication(guc);
>>   	if (ret)
>>   		goto err_log_capture;
>> @@ -562,6 +567,8 @@ static void __uc_fini_hw(struct intel_uc *uc)
>>   	if (intel_uc_uses_guc_submission(uc))
>>   		intel_guc_submission_disable(guc);
>>   
>> +	intel_guc_hwconfig_fini(&guc->hwconfig);
>> +
>>   	__uc_sanitize(uc);
>>   }
>>   
>> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
>> index 76e590fcb903..1d31e35a5154 100644
>> --- a/drivers/gpu/drm/i915/i915_pci.c
>> +++ b/drivers/gpu/drm/i915/i915_pci.c
>> @@ -990,6 +990,7 @@ static const struct intel_device_info adl_p_info = {
>>   		BIT(RCS0) | BIT(BCS0) | BIT(VECS0) | BIT(VCS0) | BIT(VCS2),
>>   	.ppgtt_size = 48,
>>   	.dma_mask_size = 39,
>> +	.has_guc_hwconfig = 1,
> Who requested this change? It was previously done this way but the 
> instruction was that i915_pci.c is for hardware features only but that 
> this, as you seem extremely keen about pointing out at every 
> opportunity, is a software feature.

This was requested by Michal as well. I definitely agree it is a
software feature, but I was not aware that "i915_pci.c is for hardware
features only".

Michal, do you agree with this and returning to the previous method for
enabling the feature?

-Jordan

^ permalink raw reply	[flat|nested] 32+ messages in thread
* [Intel-gfx] ✗ Fi.CI.BUILD: failure for GuC HWCONFIG with documentation (rev6)
  2022-02-22 10:36 ` [Intel-gfx] " Jordan Justen
                   ` (8 preceding siblings ...)
  (?)
@ 2022-02-25  7:20 ` Patchwork
  -1 siblings, 0 replies; 32+ messages in thread
From: Patchwork @ 2022-02-25  7:20 UTC (permalink / raw)
  To: john.c.harrison; +Cc: intel-gfx

== Series Details ==

Series: GuC HWCONFIG with documentation (rev6)
URL   : https://patchwork.freedesktop.org/series/99787/
State : failure

== Summary ==

Applying: drm/i915/guc: Add fetch of hwconfig table
Applying: drm/i915/uapi: Add query for hwconfig blob
Applying: drm/i915/uapi: Add struct drm_i915_query_hwconfig_blob_item
Applying: drm/i915/guc: Verify hwconfig blob matches supported format
Using index info to reconstruct a base tree...
M	drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c
Falling back to patching base and 3-way merge...
Auto-merging drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c
CONFLICT (content): Merge conflict in drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0004 drm/i915/guc: Verify hwconfig blob matches supported format
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".



^ permalink raw reply	[flat|nested] 32+ messages in thread
* Re: [PATCH v5 1/4] drm/i915/guc: Add fetch of hwconfig table
  2022-02-25  5:03       ` [Intel-gfx] " Jordan Justen
@ 2022-02-25  9:44         ` Michal Wajdeczko
  -1 siblings, 0 replies; 32+ messages in thread
From: Michal Wajdeczko @ 2022-02-25  9:44 UTC (permalink / raw)
  To: Jordan Justen, John Harrison, intel-gfx
  Cc: Matthew Brost, Jon Bloomfield, dri-devel, Rodrigo Vivi



On 25.02.2022 06:03, Jordan Justen wrote:
> John Harrison <john.c.harrison@intel.com> writes:
> 
>> On 2/22/2022 02:36, Jordan Justen wrote:
>>> From: John Harrison <John.C.Harrison@Intel.com>
>>>
>>> Implement support for fetching the hardware description table from the
>>> GuC. The call is made twice - once without a destination buffer to
>>> query the size and then a second time to fill in the buffer.
>>>
>>> Note that the table is only available on ADL-P and later platforms.
>>>
>>> v5 (of Jordan's posting):
>>>   * Various changes made by Jordan and recommended by Michal
>>>     - Makefile ordering
>>>     - Adjust "struct intel_guc_hwconfig hwconfig" comment
>>>     - Set Copyright year to 2022 in intel_guc_hwconfig.c/.h
>>>     - Drop inline from hwconfig_to_guc()
>>>     - Replace hwconfig param with guc in __guc_action_get_hwconfig()
>>>     - Move zero size check into guc_hwconfig_discover_size()
>>>     - Change comment to say zero size offset/size is needed to get size
>>>     - Add has_guc_hwconfig to devinfo and drop has_table()
>>>     - Change drm_err to notice in __uc_init_hw() and use %pe
>>>
>>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>>> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
>>> Reviewed-by: Matthew Brost <matthew.brost@intel.com>
>>> Acked-by: Jon Bloomfield <jon.bloomfield@intel.com>
>>> Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
>>> ---
>>>   
>>> +	ret = intel_guc_hwconfig_init(&guc->hwconfig);
>>> +	if (ret)
>>> +		drm_notice(&i915->drm, "Failed to retrieve hwconfig table: %pe\n",
>> Why only drm_notice? As you are keen to point out, the UMDs won't work 
>> if the table is not available. All the failure paths in your own 
>> verification function are 'drm_err'. So why is it only a 'notice' if 
>> there is no table at all?
> 
> This was requested by Michal in my v3 posting:
> 
> https://patchwork.freedesktop.org/patch/472936/?series=99787&rev=3
> 
> I don't think that it should be a failure for i915 if it is unable to
> read the table, or if the table read is invalid. I think it should be up
> to the UMD to react to the missing hwconfig however they think is
> appropriate, but I would like the i915 to guarantee & document the
> format returned to userspace to whatever extent is feasible.
> 
> As you point out there is a discrepancy, and I think I should be
> consistent with whatever is used here in my "drm/i915/guc: Verify
> hwconfig blob matches supported format" patch.
> 
> I guess I'd tend to agree with Michal that "maybe drm_notice since we
> continue probe", but I would go along with either if you two want to
> discuss further.

having consistent message level is a clear benefit but on other hand
these other 'errors' may indicate more serious problems related to use
of wrong/incompatible firmware that returns corrupted HWconfig (or we
use wrong actions), while since we are not using this HWconfig in the
driver we don't care that much that we failed to load HWconfig and
'notice' is enough.

but I'm fine with all messages being drm_err (as we will not have to
change that once again after HWconfig will be mandatory for the driver
as well)

> 
>> Note that this function is called as part of the reset path. The reset 
>> path is not allowed to allocate memory. The table is stored in a 
>> dynamically allocated object. Hence the IGT test failure. The table 
>> query has to be done elsewhere at driver init time only.
> 
> Thanks for clearing this up. I did notice on dg2 that gpu resets were
> causing a re-read of the hwconfig from GuC, but it definitely was not
> clear to me that there would be a connection to the IGT failure that you
> pointed out.
> 
>>
>>> +			   ERR_PTR(ret));
>>> +
>>>   	ret = guc_enable_communication(guc);
>>>   	if (ret)
>>>   		goto err_log_capture;
>>> @@ -562,6 +567,8 @@ static void __uc_fini_hw(struct intel_uc *uc)
>>>   	if (intel_uc_uses_guc_submission(uc))
>>>   		intel_guc_submission_disable(guc);
>>>   
>>> +	intel_guc_hwconfig_fini(&guc->hwconfig);
>>> +
>>>   	__uc_sanitize(uc);
>>>   }
>>>   
>>> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
>>> index 76e590fcb903..1d31e35a5154 100644
>>> --- a/drivers/gpu/drm/i915/i915_pci.c
>>> +++ b/drivers/gpu/drm/i915/i915_pci.c
>>> @@ -990,6 +990,7 @@ static const struct intel_device_info adl_p_info = {
>>>   		BIT(RCS0) | BIT(BCS0) | BIT(VECS0) | BIT(VCS0) | BIT(VCS2),
>>>   	.ppgtt_size = 48,
>>>   	.dma_mask_size = 39,
>>> +	.has_guc_hwconfig = 1,
>> Who requested this change? It was previously done this way but the 
>> instruction was that i915_pci.c is for hardware features only but that 
>> this, as you seem extremely keen about pointing out at every 
>> opportunity, is a software feature.
> 
> This was requested by Michal as well. I definitely agree it is a
> software feature, but I was not aware that "i915_pci.c is for hardware
> features only".
> 
> Michal, do you agree with this and returning to the previous method for
> enabling the feature?

now I'm little confused as some arch direction was to treat FW as
extension of the HW so for me it was natural to have 'has_guc_hwconfig'
flag in device_info

if still for some reason it is undesired to mix HW and FW/SW flags
inside single group of flags then maybe we should just add separate
group of immutable flags where has_guc_hwconfig could be defined.

let our maintainers decide

Michal

> 
> -Jordan

^ permalink raw reply	[flat|nested] 32+ messages in thread
* Re: [Intel-gfx] [PATCH v5 1/4] drm/i915/guc: Add fetch of hwconfig table
@ 2022-02-25  9:44         ` Michal Wajdeczko
  0 siblings, 0 replies; 32+ messages in thread
From: Michal Wajdeczko @ 2022-02-25  9:44 UTC (permalink / raw)
  To: Jordan Justen, John Harrison, intel-gfx; +Cc: dri-devel, Rodrigo Vivi



On 25.02.2022 06:03, Jordan Justen wrote:
> John Harrison <john.c.harrison@intel.com> writes:
> 
>> On 2/22/2022 02:36, Jordan Justen wrote:
>>> From: John Harrison <John.C.Harrison@Intel.com>
>>>
>>> Implement support for fetching the hardware description table from the
>>> GuC. The call is made twice - once without a destination buffer to
>>> query the size and then a second time to fill in the buffer.
>>>
>>> Note that the table is only available on ADL-P and later platforms.
>>>
>>> v5 (of Jordan's posting):
>>>   * Various changes made by Jordan and recommended by Michal
>>>     - Makefile ordering
>>>     - Adjust "struct intel_guc_hwconfig hwconfig" comment
>>>     - Set Copyright year to 2022 in intel_guc_hwconfig.c/.h
>>>     - Drop inline from hwconfig_to_guc()
>>>     - Replace hwconfig param with guc in __guc_action_get_hwconfig()
>>>     - Move zero size check into guc_hwconfig_discover_size()
>>>     - Change comment to say zero size offset/size is needed to get size
>>>     - Add has_guc_hwconfig to devinfo and drop has_table()
>>>     - Change drm_err to notice in __uc_init_hw() and use %pe
>>>
>>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>>> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
>>> Reviewed-by: Matthew Brost <matthew.brost@intel.com>
>>> Acked-by: Jon Bloomfield <jon.bloomfield@intel.com>
>>> Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
>>> ---
>>>   
>>> +	ret = intel_guc_hwconfig_init(&guc->hwconfig);
>>> +	if (ret)
>>> +		drm_notice(&i915->drm, "Failed to retrieve hwconfig table: %pe\n",
>> Why only drm_notice? As you are keen to point out, the UMDs won't work 
>> if the table is not available. All the failure paths in your own 
>> verification function are 'drm_err'. So why is it only a 'notice' if 
>> there is no table at all?
> 
> This was requested by Michal in my v3 posting:
> 
> https://patchwork.freedesktop.org/patch/472936/?series=99787&rev=3
> 
> I don't think that it should be a failure for i915 if it is unable to
> read the table, or if the table read is invalid. I think it should be up
> to the UMD to react to the missing hwconfig however they think is
> appropriate, but I would like the i915 to guarantee & document the
> format returned to userspace to whatever extent is feasible.
> 
> As you point out there is a discrepancy, and I think I should be
> consistent with whatever is used here in my "drm/i915/guc: Verify
> hwconfig blob matches supported format" patch.
> 
> I guess I'd tend to agree with Michal that "maybe drm_notice since we
> continue probe", but I would go along with either if you two want to
> discuss further.

having consistent message level is a clear benefit but on other hand
these other 'errors' may indicate more serious problems related to use
of wrong/incompatible firmware that returns corrupted HWconfig (or we
use wrong actions), while since we are not using this HWconfig in the
driver we don't care that much that we failed to load HWconfig and
'notice' is enough.

but I'm fine with all messages being drm_err (as we will not have to
change that once again after HWconfig will be mandatory for the driver
as well)

> 
>> Note that this function is called as part of the reset path. The reset 
>> path is not allowed to allocate memory. The table is stored in a 
>> dynamically allocated object. Hence the IGT test failure. The table 
>> query has to be done elsewhere at driver init time only.
> 
> Thanks for clearing this up. I did notice on dg2 that gpu resets were
> causing a re-read of the hwconfig from GuC, but it definitely was not
> clear to me that there would be a connection to the IGT failure that you
> pointed out.
> 
>>
>>> +			   ERR_PTR(ret));
>>> +
>>>   	ret = guc_enable_communication(guc);
>>>   	if (ret)
>>>   		goto err_log_capture;
>>> @@ -562,6 +567,8 @@ static void __uc_fini_hw(struct intel_uc *uc)
>>>   	if (intel_uc_uses_guc_submission(uc))
>>>   		intel_guc_submission_disable(guc);
>>>   
>>> +	intel_guc_hwconfig_fini(&guc->hwconfig);
>>> +
>>>   	__uc_sanitize(uc);
>>>   }
>>>   
>>> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
>>> index 76e590fcb903..1d31e35a5154 100644
>>> --- a/drivers/gpu/drm/i915/i915_pci.c
>>> +++ b/drivers/gpu/drm/i915/i915_pci.c
>>> @@ -990,6 +990,7 @@ static const struct intel_device_info adl_p_info = {
>>>   		BIT(RCS0) | BIT(BCS0) | BIT(VECS0) | BIT(VCS0) | BIT(VCS2),
>>>   	.ppgtt_size = 48,
>>>   	.dma_mask_size = 39,
>>> +	.has_guc_hwconfig = 1,
>> Who requested this change? It was previously done this way but the 
>> instruction was that i915_pci.c is for hardware features only but that 
>> this, as you seem extremely keen about pointing out at every 
>> opportunity, is a software feature.
> 
> This was requested by Michal as well. I definitely agree it is a
> software feature, but I was not aware that "i915_pci.c is for hardware
> features only".
> 
> Michal, do you agree with this and returning to the previous method for
> enabling the feature?

now I'm little confused as some arch direction was to treat FW as
extension of the HW so for me it was natural to have 'has_guc_hwconfig'
flag in device_info

if still for some reason it is undesired to mix HW and FW/SW flags
inside single group of flags then maybe we should just add separate
group of immutable flags where has_guc_hwconfig could be defined.

let our maintainers decide

Michal

> 
> -Jordan

^ permalink raw reply	[flat|nested] 32+ messages in thread
* Re: [Intel-gfx] [PATCH v5 1/4] drm/i915/guc: Add fetch of hwconfig table
  2022-02-25  9:44         ` [Intel-gfx] " Michal Wajdeczko
  (?)
@ 2022-02-25 13:26         ` Tvrtko Ursulin
  2022-02-25 16:46           ` John Harrison
  -1 siblings, 1 reply; 32+ messages in thread
From: Tvrtko Ursulin @ 2022-02-25 13:26 UTC (permalink / raw)
  To: Michal Wajdeczko, Jordan Justen, John Harrison, intel-gfx
  Cc: dri-devel, Rodrigo Vivi


On 25/02/2022 09:44, Michal Wajdeczko wrote:
> On 25.02.2022 06:03, Jordan Justen wrote:
>> John Harrison <john.c.harrison@intel.com> writes:
>>
>>> On 2/22/2022 02:36, Jordan Justen wrote:
>>>> From: John Harrison <John.C.Harrison@Intel.com>
>>>>
>>>> Implement support for fetching the hardware description table from the
>>>> GuC. The call is made twice - once without a destination buffer to
>>>> query the size and then a second time to fill in the buffer.
>>>>
>>>> Note that the table is only available on ADL-P and later platforms.
>>>>
>>>> v5 (of Jordan's posting):
>>>>    * Various changes made by Jordan and recommended by Michal
>>>>      - Makefile ordering
>>>>      - Adjust "struct intel_guc_hwconfig hwconfig" comment
>>>>      - Set Copyright year to 2022 in intel_guc_hwconfig.c/.h
>>>>      - Drop inline from hwconfig_to_guc()
>>>>      - Replace hwconfig param with guc in __guc_action_get_hwconfig()
>>>>      - Move zero size check into guc_hwconfig_discover_size()
>>>>      - Change comment to say zero size offset/size is needed to get size
>>>>      - Add has_guc_hwconfig to devinfo and drop has_table()
>>>>      - Change drm_err to notice in __uc_init_hw() and use %pe
>>>>
>>>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>>>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>>>> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
>>>> Reviewed-by: Matthew Brost <matthew.brost@intel.com>
>>>> Acked-by: Jon Bloomfield <jon.bloomfield@intel.com>
>>>> Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
>>>> ---
>>>>    
>>>> +	ret = intel_guc_hwconfig_init(&guc->hwconfig);
>>>> +	if (ret)
>>>> +		drm_notice(&i915->drm, "Failed to retrieve hwconfig table: %pe\n",
>>> Why only drm_notice? As you are keen to point out, the UMDs won't work
>>> if the table is not available. All the failure paths in your own
>>> verification function are 'drm_err'. So why is it only a 'notice' if
>>> there is no table at all?
>>
>> This was requested by Michal in my v3 posting:
>>
>> https://patchwork.freedesktop.org/patch/472936/?series=99787&rev=3
>>
>> I don't think that it should be a failure for i915 if it is unable to
>> read the table, or if the table read is invalid. I think it should be up
>> to the UMD to react to the missing hwconfig however they think is
>> appropriate, but I would like the i915 to guarantee & document the
>> format returned to userspace to whatever extent is feasible.
>>
>> As you point out there is a discrepancy, and I think I should be
>> consistent with whatever is used here in my "drm/i915/guc: Verify
>> hwconfig blob matches supported format" patch.
>>
>> I guess I'd tend to agree with Michal that "maybe drm_notice since we
>> continue probe", but I would go along with either if you two want to
>> discuss further.
> 
> having consistent message level is a clear benefit but on other hand
> these other 'errors' may indicate more serious problems related to use
> of wrong/incompatible firmware that returns corrupted HWconfig (or we
> use wrong actions), while since we are not using this HWconfig in the
> driver we don't care that much that we failed to load HWconfig and
> 'notice' is enough.
> 
> but I'm fine with all messages being drm_err (as we will not have to
> change that once again after HWconfig will be mandatory for the driver
> as well)

I would be against drm_err.

#define KERN_EMERG      KERN_SOH "0"    /* system is unusable */
#define KERN_ALERT      KERN_SOH "1"    /* action must be taken immediately */
#define KERN_CRIT       KERN_SOH "2"    /* critical conditions */
#define KERN_ERR        KERN_SOH "3"    /* error conditions */
#define KERN_WARNING    KERN_SOH "4"    /* warning conditions */
#define KERN_NOTICE     KERN_SOH "5"    /* normal but significant condition */
#define KERN_INFO       KERN_SOH "6"    /* informational */
#define KERN_DEBUG      KERN_SOH "7"    /* debug-level messages */

 From the point of view of the kernel driver, this is not an error to its operation. It can at most be a warning, but notice is also fine by me. One could argue when reading "normal but significant condition" that it is not normal, when it is in fact unexpected, so if people prefer warning that is also okay by me. I still lean towards notice becuase of the hands-off nature i915 has with the pass-through of this blob.

>>> Note that this function is called as part of the reset path. The reset
>>> path is not allowed to allocate memory. The table is stored in a
>>> dynamically allocated object. Hence the IGT test failure. The table
>>> query has to be done elsewhere at driver init time only.
>>
>> Thanks for clearing this up. I did notice on dg2 that gpu resets were
>> causing a re-read of the hwconfig from GuC, but it definitely was not
>> clear to me that there would be a connection to the IGT failure that you
>> pointed out.
>>
>>>
>>>> +			   ERR_PTR(ret));
>>>> +
>>>>    	ret = guc_enable_communication(guc);
>>>>    	if (ret)
>>>>    		goto err_log_capture;
>>>> @@ -562,6 +567,8 @@ static void __uc_fini_hw(struct intel_uc *uc)
>>>>    	if (intel_uc_uses_guc_submission(uc))
>>>>    		intel_guc_submission_disable(guc);
>>>>    
>>>> +	intel_guc_hwconfig_fini(&guc->hwconfig);
>>>> +
>>>>    	__uc_sanitize(uc);
>>>>    }
>>>>    
>>>> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
>>>> index 76e590fcb903..1d31e35a5154 100644
>>>> --- a/drivers/gpu/drm/i915/i915_pci.c
>>>> +++ b/drivers/gpu/drm/i915/i915_pci.c
>>>> @@ -990,6 +990,7 @@ static const struct intel_device_info adl_p_info = {
>>>>    		BIT(RCS0) | BIT(BCS0) | BIT(VECS0) | BIT(VCS0) | BIT(VCS2),
>>>>    	.ppgtt_size = 48,
>>>>    	.dma_mask_size = 39,
>>>> +	.has_guc_hwconfig = 1,
>>> Who requested this change? It was previously done this way but the
>>> instruction was that i915_pci.c is for hardware features only but that
>>> this, as you seem extremely keen about pointing out at every
>>> opportunity, is a software feature.
>>
>> This was requested by Michal as well. I definitely agree it is a
>> software feature, but I was not aware that "i915_pci.c is for hardware
>> features only".
>>
>> Michal, do you agree with this and returning to the previous method for
>> enabling the feature?
> 
> now I'm little confused as some arch direction was to treat FW as
> extension of the HW so for me it was natural to have 'has_guc_hwconfig'
> flag in device_info
> 
> if still for some reason it is undesired to mix HW and FW/SW flags
> inside single group of flags then maybe we should just add separate
> group of immutable flags where has_guc_hwconfig could be defined.
> 
> let our maintainers decide

Bah.. :)

And what was the previous method?

[comes back later]

Okay it was:

+static bool has_table(struct drm_i915_private *i915)
+{
+	if (IS_ALDERLAKE_P(i915))
+		return true;

Which sucks a bit if we want to argue it does not belong in device info.

Why can't we ask the GuC if the blob exists? In fact what would happen if one would call __guc_action_get_hwconfig on any GuC platform?

Regards,

Tvrtko

^ permalink raw reply	[flat|nested] 32+ messages in thread
* Re: [Intel-gfx] [PATCH v5 1/4] drm/i915/guc: Add fetch of hwconfig table
  2022-02-25 13:26         ` Tvrtko Ursulin
@ 2022-02-25 16:46           ` John Harrison
  2022-02-25 17:18             ` Tvrtko Ursulin
  0 siblings, 1 reply; 32+ messages in thread
From: John Harrison @ 2022-02-25 16:46 UTC (permalink / raw)
  To: Tvrtko Ursulin, Michal Wajdeczko, Jordan Justen, intel-gfx
  Cc: dri-devel, Rodrigo Vivi

On 2/25/2022 05:26, Tvrtko Ursulin wrote:
> On 25/02/2022 09:44, Michal Wajdeczko wrote:
>> On 25.02.2022 06:03, Jordan Justen wrote:
>>> John Harrison <john.c.harrison@intel.com> writes:
>>>
>>>> On 2/22/2022 02:36, Jordan Justen wrote:
>>>>> From: John Harrison <John.C.Harrison@Intel.com>
>>>>>
>>>>> Implement support for fetching the hardware description table from 
>>>>> the
>>>>> GuC. The call is made twice - once without a destination buffer to
>>>>> query the size and then a second time to fill in the buffer.
>>>>>
>>>>> Note that the table is only available on ADL-P and later platforms.
>>>>>
>>>>> v5 (of Jordan's posting):
>>>>>    * Various changes made by Jordan and recommended by Michal
>>>>>      - Makefile ordering
>>>>>      - Adjust "struct intel_guc_hwconfig hwconfig" comment
>>>>>      - Set Copyright year to 2022 in intel_guc_hwconfig.c/.h
>>>>>      - Drop inline from hwconfig_to_guc()
>>>>>      - Replace hwconfig param with guc in __guc_action_get_hwconfig()
>>>>>      - Move zero size check into guc_hwconfig_discover_size()
>>>>>      - Change comment to say zero size offset/size is needed to 
>>>>> get size
>>>>>      - Add has_guc_hwconfig to devinfo and drop has_table()
>>>>>      - Change drm_err to notice in __uc_init_hw() and use %pe
>>>>>
>>>>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>>>>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>>>>> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
>>>>> Reviewed-by: Matthew Brost <matthew.brost@intel.com>
>>>>> Acked-by: Jon Bloomfield <jon.bloomfield@intel.com>
>>>>> Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
>>>>> ---
>>>>>    +    ret = intel_guc_hwconfig_init(&guc->hwconfig);
>>>>> +    if (ret)
>>>>> +        drm_notice(&i915->drm, "Failed to retrieve hwconfig 
>>>>> table: %pe\n",
>>>> Why only drm_notice? As you are keen to point out, the UMDs won't work
>>>> if the table is not available. All the failure paths in your own
>>>> verification function are 'drm_err'. So why is it only a 'notice' if
>>>> there is no table at all?
>>>
>>> This was requested by Michal in my v3 posting:
>>>
>>> https://patchwork.freedesktop.org/patch/472936/?series=99787&rev=3
>>>
>>> I don't think that it should be a failure for i915 if it is unable to
>>> read the table, or if the table read is invalid. I think it should 
>>> be up
>>> to the UMD to react to the missing hwconfig however they think is
>>> appropriate, but I would like the i915 to guarantee & document the
>>> format returned to userspace to whatever extent is feasible.
>>>
>>> As you point out there is a discrepancy, and I think I should be
>>> consistent with whatever is used here in my "drm/i915/guc: Verify
>>> hwconfig blob matches supported format" patch.
>>>
>>> I guess I'd tend to agree with Michal that "maybe drm_notice since we
>>> continue probe", but I would go along with either if you two want to
>>> discuss further.
>>
>> having consistent message level is a clear benefit but on other hand
>> these other 'errors' may indicate more serious problems related to use
>> of wrong/incompatible firmware that returns corrupted HWconfig (or we
>> use wrong actions), while since we are not using this HWconfig in the
As stated ad nauseam, you can rule out 'corrupted' hwconfig. The GuC 
firmware is signed and will not load if it has become corrupted somehow. 
Likewise, a 'wrong/incompatible' firmware will refuse to load. So it is 
physically impossible for the later verification stage to ever find an 
error.


>> driver we don't care that much that we failed to load HWconfig and
>> 'notice' is enough.
>>
>> but I'm fine with all messages being drm_err (as we will not have to
>> change that once again after HWconfig will be mandatory for the driver
>> as well)
>
> I would be against drm_err.
>
> #define KERN_EMERG      KERN_SOH "0"    /* system is unusable */
> #define KERN_ALERT      KERN_SOH "1"    /* action must be taken 
> immediately */
> #define KERN_CRIT       KERN_SOH "2"    /* critical conditions */
> #define KERN_ERR        KERN_SOH "3"    /* error conditions */
> #define KERN_WARNING    KERN_SOH "4"    /* warning conditions */
> #define KERN_NOTICE     KERN_SOH "5"    /* normal but significant 
> condition */
> #define KERN_INFO       KERN_SOH "6"    /* informational */
> #define KERN_DEBUG      KERN_SOH "7"    /* debug-level messages */
>
> From the point of view of the kernel driver, this is not an error to 
> its operation. It can at most be a warning, but notice is also fine by 
> me. One could argue when reading "normal but significant condition" 
> that it is not normal, when it is in fact unexpected, so if people 
> prefer warning that is also okay by me. I still lean towards notice 
> becuase of the hands-off nature i915 has with the pass-through of this 
> blob.
 From the point of view of the KMD, i915 will load and be 'functional' 
if it can't talk to the hardware at all. The UMDs won't work at all but 
the driver load will be 'fine'. That's a requirement to be able to get 
the user to a software fallback desktop in order to work out why the 
hardware isn't working (e.g. no GuC firmware file). I would view this as 
similar. The KMD might have loaded but the UMDs are not functional. That 
is definitely an error condition to me.

>
>>>> Note that this function is called as part of the reset path. The reset
>>>> path is not allowed to allocate memory. The table is stored in a
>>>> dynamically allocated object. Hence the IGT test failure. The table
>>>> query has to be done elsewhere at driver init time only.
>>>
>>> Thanks for clearing this up. I did notice on dg2 that gpu resets were
>>> causing a re-read of the hwconfig from GuC, but it definitely was not
>>> clear to me that there would be a connection to the IGT failure that 
>>> you
>>> pointed out.
>>>
>>>>
>>>>> +               ERR_PTR(ret));
>>>>> +
>>>>>        ret = guc_enable_communication(guc);
>>>>>        if (ret)
>>>>>            goto err_log_capture;
>>>>> @@ -562,6 +567,8 @@ static void __uc_fini_hw(struct intel_uc *uc)
>>>>>        if (intel_uc_uses_guc_submission(uc))
>>>>>            intel_guc_submission_disable(guc);
>>>>>    +    intel_guc_hwconfig_fini(&guc->hwconfig);
>>>>> +
>>>>>        __uc_sanitize(uc);
>>>>>    }
>>>>>    diff --git a/drivers/gpu/drm/i915/i915_pci.c 
>>>>> b/drivers/gpu/drm/i915/i915_pci.c
>>>>> index 76e590fcb903..1d31e35a5154 100644
>>>>> --- a/drivers/gpu/drm/i915/i915_pci.c
>>>>> +++ b/drivers/gpu/drm/i915/i915_pci.c
>>>>> @@ -990,6 +990,7 @@ static const struct intel_device_info 
>>>>> adl_p_info = {
>>>>>            BIT(RCS0) | BIT(BCS0) | BIT(VECS0) | BIT(VCS0) | 
>>>>> BIT(VCS2),
>>>>>        .ppgtt_size = 48,
>>>>>        .dma_mask_size = 39,
>>>>> +    .has_guc_hwconfig = 1,
>>>> Who requested this change? It was previously done this way but the
>>>> instruction was that i915_pci.c is for hardware features only but that
>>>> this, as you seem extremely keen about pointing out at every
>>>> opportunity, is a software feature.
>>>
>>> This was requested by Michal as well. I definitely agree it is a
>>> software feature, but I was not aware that "i915_pci.c is for hardware
>>> features only".
>>>
>>> Michal, do you agree with this and returning to the previous method for
>>> enabling the feature?
>>
>> now I'm little confused as some arch direction was to treat FW as
>> extension of the HW so for me it was natural to have 'has_guc_hwconfig'
>> flag in device_info
>>
>> if still for some reason it is undesired to mix HW and FW/SW flags
>> inside single group of flags then maybe we should just add separate
>> group of immutable flags where has_guc_hwconfig could be defined.
>>
>> let our maintainers decide
>
> Bah.. :)
>
> And what was the previous method?
>
> [comes back later]
>
> Okay it was:
>
> +static bool has_table(struct drm_i915_private *i915)
> +{
> +    if (IS_ALDERLAKE_P(i915))
> +        return true;
>
> Which sucks a bit if we want to argue it does not belong in device info.
>
> Why can't we ask the GuC if the blob exists? In fact what would happen 
> if one would call __guc_action_get_hwconfig on any GuC platform?
That was how I originally wrote the code. However, other parties refuse 
to allow a H2G call to fail. The underlying CTB layers complain loudly 
on any CTB error. And the GuC architects insist that a call to query the 
table on an unsupported platform is an error and should return an 
'unsupported' error code.

John.

>
> Regards,
>
> Tvrtko


^ permalink raw reply	[flat|nested] 32+ messages in thread
* Re: [Intel-gfx] [PATCH v5 1/4] drm/i915/guc: Add fetch of hwconfig table
  2022-02-25 16:46           ` John Harrison
@ 2022-02-25 17:18             ` Tvrtko Ursulin
  2022-02-25 18:05               ` Michal Wajdeczko
  0 siblings, 1 reply; 32+ messages in thread
From: Tvrtko Ursulin @ 2022-02-25 17:18 UTC (permalink / raw)
  To: John Harrison, Michal Wajdeczko, Jordan Justen, intel-gfx
  Cc: dri-devel, Rodrigo Vivi


On 25/02/2022 16:46, John Harrison wrote:

>>> driver we don't care that much that we failed to load HWconfig and
>>> 'notice' is enough.
>>>
>>> but I'm fine with all messages being drm_err (as we will not have to
>>> change that once again after HWconfig will be mandatory for the driver
>>> as well)
>>
>> I would be against drm_err.
>>
>> #define KERN_EMERG      KERN_SOH "0"    /* system is unusable */
>> #define KERN_ALERT      KERN_SOH "1"    /* action must be taken 
>> immediately */
>> #define KERN_CRIT       KERN_SOH "2"    /* critical conditions */
>> #define KERN_ERR        KERN_SOH "3"    /* error conditions */
>> #define KERN_WARNING    KERN_SOH "4"    /* warning conditions */
>> #define KERN_NOTICE     KERN_SOH "5"    /* normal but significant 
>> condition */
>> #define KERN_INFO       KERN_SOH "6"    /* informational */
>> #define KERN_DEBUG      KERN_SOH "7"    /* debug-level messages */
>>
>> From the point of view of the kernel driver, this is not an error to 
>> its operation. It can at most be a warning, but notice is also fine by 
>> me. One could argue when reading "normal but significant condition" 
>> that it is not normal, when it is in fact unexpected, so if people 
>> prefer warning that is also okay by me. I still lean towards notice 
>> becuase of the hands-off nature i915 has with the pass-through of this 
>> blob.
>  From the point of view of the KMD, i915 will load and be 'functional' 
> if it can't talk to the hardware at all. The UMDs won't work at all but 

Well this reductio ad absurdum fails I think... :)

> the driver load will be 'fine'. That's a requirement to be able to get 
> the user to a software fallback desktop in order to work out why the 
> hardware isn't working (e.g. no GuC firmware file). I would view this as 
> similar. The KMD might have loaded but the UMDs are not functional. That 
> is definitely an error condition to me.

... If GuC fails to load there is no command submission and driver will 
obviously log that with drm_err.

If blob fails to verify it depends on the userspace stack what will 
happen. As stated before on some platforms, and/or after a certain time, 
Mesa will not look at the blob at all. So i915 is fine (it is after all 
just a conduit for opaque data!), system overall is fine, so it 
definitely isn't a KERN_ERR level event.

>>>>>> +               ERR_PTR(ret));
>>>>>> +
>>>>>>        ret = guc_enable_communication(guc);
>>>>>>        if (ret)
>>>>>>            goto err_log_capture;
>>>>>> @@ -562,6 +567,8 @@ static void __uc_fini_hw(struct intel_uc *uc)
>>>>>>        if (intel_uc_uses_guc_submission(uc))
>>>>>>            intel_guc_submission_disable(guc);
>>>>>>    +    intel_guc_hwconfig_fini(&guc->hwconfig);
>>>>>> +
>>>>>>        __uc_sanitize(uc);
>>>>>>    }
>>>>>>    diff --git a/drivers/gpu/drm/i915/i915_pci.c 
>>>>>> b/drivers/gpu/drm/i915/i915_pci.c
>>>>>> index 76e590fcb903..1d31e35a5154 100644
>>>>>> --- a/drivers/gpu/drm/i915/i915_pci.c
>>>>>> +++ b/drivers/gpu/drm/i915/i915_pci.c
>>>>>> @@ -990,6 +990,7 @@ static const struct intel_device_info 
>>>>>> adl_p_info = {
>>>>>>            BIT(RCS0) | BIT(BCS0) | BIT(VECS0) | BIT(VCS0) | 
>>>>>> BIT(VCS2),
>>>>>>        .ppgtt_size = 48,
>>>>>>        .dma_mask_size = 39,
>>>>>> +    .has_guc_hwconfig = 1,
>>>>> Who requested this change? It was previously done this way but the
>>>>> instruction was that i915_pci.c is for hardware features only but that
>>>>> this, as you seem extremely keen about pointing out at every
>>>>> opportunity, is a software feature.
>>>>
>>>> This was requested by Michal as well. I definitely agree it is a
>>>> software feature, but I was not aware that "i915_pci.c is for hardware
>>>> features only".
>>>>
>>>> Michal, do you agree with this and returning to the previous method for
>>>> enabling the feature?
>>>
>>> now I'm little confused as some arch direction was to treat FW as
>>> extension of the HW so for me it was natural to have 'has_guc_hwconfig'
>>> flag in device_info
>>>
>>> if still for some reason it is undesired to mix HW and FW/SW flags
>>> inside single group of flags then maybe we should just add separate
>>> group of immutable flags where has_guc_hwconfig could be defined.
>>>
>>> let our maintainers decide
>>
>> Bah.. :)
>>
>> And what was the previous method?
>>
>> [comes back later]
>>
>> Okay it was:
>>
>> +static bool has_table(struct drm_i915_private *i915)
>> +{
>> +    if (IS_ALDERLAKE_P(i915))
>> +        return true;
>>
>> Which sucks a bit if we want to argue it does not belong in device info.
>>
>> Why can't we ask the GuC if the blob exists? In fact what would happen 
>> if one would call __guc_action_get_hwconfig on any GuC platform?
> That was how I originally wrote the code. However, other parties refuse 
> to allow a H2G call to fail. The underlying CTB layers complain loudly 
> on any CTB error. And the GuC architects insist that a call to query the 
> table on an unsupported platform is an error and should return an 
> 'unsupported' error code.

Oh well, shrug, sounds silly but I will not pretend I am familiar with H2G

In this case has_table does sound better since it indeed isn't a 
hardware feature. It is a GuC fw thing and if we don't have a way to 
probe things there at runtime, then at least limit the knowledge to GuC 
files.

Regards,

Tvrtko

^ permalink raw reply	[flat|nested] 32+ messages in thread
* Re: [Intel-gfx] [PATCH v5 1/4] drm/i915/guc: Add fetch of hwconfig table
  2022-02-25 17:18             ` Tvrtko Ursulin
@ 2022-02-25 18:05               ` Michal Wajdeczko
  2022-02-25 18:35                 ` Tvrtko Ursulin
  0 siblings, 1 reply; 32+ messages in thread
From: Michal Wajdeczko @ 2022-02-25 18:05 UTC (permalink / raw)
  To: Tvrtko Ursulin, John Harrison, Jordan Justen, intel-gfx
  Cc: dri-devel, Rodrigo Vivi



On 25.02.2022 18:18, Tvrtko Ursulin wrote:
> 
> On 25/02/2022 16:46, John Harrison wrote:
> 
>>>> driver we don't care that much that we failed to load HWconfig and
>>>> 'notice' is enough.
>>>>
>>>> but I'm fine with all messages being drm_err (as we will not have to
>>>> change that once again after HWconfig will be mandatory for the driver
>>>> as well)
>>>
>>> I would be against drm_err.
>>>
>>> #define KERN_EMERG      KERN_SOH "0"    /* system is unusable */
>>> #define KERN_ALERT      KERN_SOH "1"    /* action must be taken
>>> immediately */
>>> #define KERN_CRIT       KERN_SOH "2"    /* critical conditions */
>>> #define KERN_ERR        KERN_SOH "3"    /* error conditions */
>>> #define KERN_WARNING    KERN_SOH "4"    /* warning conditions */
>>> #define KERN_NOTICE     KERN_SOH "5"    /* normal but significant
>>> condition */
>>> #define KERN_INFO       KERN_SOH "6"    /* informational */
>>> #define KERN_DEBUG      KERN_SOH "7"    /* debug-level messages */
>>>
>>> From the point of view of the kernel driver, this is not an error to
>>> its operation. It can at most be a warning, but notice is also fine
>>> by me. One could argue when reading "normal but significant
>>> condition" that it is not normal, when it is in fact unexpected, so
>>> if people prefer warning that is also okay by me. I still lean
>>> towards notice becuase of the hands-off nature i915 has with the
>>> pass-through of this blob.
>>  From the point of view of the KMD, i915 will load and be 'functional'
>> if it can't talk to the hardware at all. The UMDs won't work at all but 
> 
> Well this reductio ad absurdum fails I think... :)
> 
>> the driver load will be 'fine'. That's a requirement to be able to get
>> the user to a software fallback desktop in order to work out why the
>> hardware isn't working (e.g. no GuC firmware file). I would view this
>> as similar. The KMD might have loaded but the UMDs are not functional.
>> That is definitely an error condition to me.
> 
> ... If GuC fails to load there is no command submission and driver will
> obviously log that with drm_err.
> 
> If blob fails to verify it depends on the userspace stack what will
> happen. As stated before on some platforms, and/or after a certain time,
> Mesa will not look at the blob at all. So i915 is fine (it is after all
> just a conduit for opaque data!), system overall is fine, so it
> definitely isn't a KERN_ERR level event.
> 
>>>>>>> +               ERR_PTR(ret));
>>>>>>> +
>>>>>>>        ret = guc_enable_communication(guc);
>>>>>>>        if (ret)
>>>>>>>            goto err_log_capture;
>>>>>>> @@ -562,6 +567,8 @@ static void __uc_fini_hw(struct intel_uc *uc)
>>>>>>>        if (intel_uc_uses_guc_submission(uc))
>>>>>>>            intel_guc_submission_disable(guc);
>>>>>>>    +    intel_guc_hwconfig_fini(&guc->hwconfig);
>>>>>>> +
>>>>>>>        __uc_sanitize(uc);
>>>>>>>    }
>>>>>>>    diff --git a/drivers/gpu/drm/i915/i915_pci.c
>>>>>>> b/drivers/gpu/drm/i915/i915_pci.c
>>>>>>> index 76e590fcb903..1d31e35a5154 100644
>>>>>>> --- a/drivers/gpu/drm/i915/i915_pci.c
>>>>>>> +++ b/drivers/gpu/drm/i915/i915_pci.c
>>>>>>> @@ -990,6 +990,7 @@ static const struct intel_device_info
>>>>>>> adl_p_info = {
>>>>>>>            BIT(RCS0) | BIT(BCS0) | BIT(VECS0) | BIT(VCS0) |
>>>>>>> BIT(VCS2),
>>>>>>>        .ppgtt_size = 48,
>>>>>>>        .dma_mask_size = 39,
>>>>>>> +    .has_guc_hwconfig = 1,
>>>>>> Who requested this change? It was previously done this way but the
>>>>>> instruction was that i915_pci.c is for hardware features only but
>>>>>> that
>>>>>> this, as you seem extremely keen about pointing out at every
>>>>>> opportunity, is a software feature.
>>>>>
>>>>> This was requested by Michal as well. I definitely agree it is a
>>>>> software feature, but I was not aware that "i915_pci.c is for hardware
>>>>> features only".
>>>>>
>>>>> Michal, do you agree with this and returning to the previous method
>>>>> for
>>>>> enabling the feature?
>>>>
>>>> now I'm little confused as some arch direction was to treat FW as
>>>> extension of the HW so for me it was natural to have 'has_guc_hwconfig'
>>>> flag in device_info
>>>>
>>>> if still for some reason it is undesired to mix HW and FW/SW flags
>>>> inside single group of flags then maybe we should just add separate
>>>> group of immutable flags where has_guc_hwconfig could be defined.
>>>>
>>>> let our maintainers decide
>>>
>>> Bah.. :)
>>>
>>> And what was the previous method?
>>>
>>> [comes back later]
>>>
>>> Okay it was:
>>>
>>> +static bool has_table(struct drm_i915_private *i915)
>>> +{
>>> +    if (IS_ALDERLAKE_P(i915))
>>> +        return true;
>>>
>>> Which sucks a bit if we want to argue it does not belong in device info.
>>>
>>> Why can't we ask the GuC if the blob exists? In fact what would
>>> happen if one would call __guc_action_get_hwconfig on any GuC platform?
>> That was how I originally wrote the code. However, other parties
>> refuse to allow a H2G call to fail. The underlying CTB layers complain
>> loudly on any CTB error. And the GuC architects insist that a call to
>> query the table on an unsupported platform is an error and should
>> return an 'unsupported' error code.

just for the background: IIRC the reason why arch didn't want 'query'
mode that wont fail was that HWconfig is not optional on these selected
platforms, so driver shall know up-front on which platforms it shall be
working (and driver shall even fail if blob is not available)

> 
> Oh well, shrug, sounds silly but I will not pretend I am familiar with H2G
> 
> In this case has_table does sound better since it indeed isn't a
> hardware feature. It is a GuC fw thing and if we don't have a way to
> probe things there at runtime, then at least limit the knowledge to GuC
> files.

note that arch expectation is that driver itself shall be using this
hwconfig (ie. will decode required data). while current approach is that
driver acts only as a proxy to UMD, this has to change and refactoring
will start (likely) soon. therefore flag has_guc_hwconfig fits better
IMHO as this wont be just 'GuC' fw thing.

Michal

> 
> Regards,
> 
> Tvrtko

^ permalink raw reply	[flat|nested] 32+ messages in thread
* Re: [Intel-gfx] [PATCH v5 1/4] drm/i915/guc: Add fetch of hwconfig table
  2022-02-25 18:05               ` Michal Wajdeczko
@ 2022-02-25 18:35                 ` Tvrtko Ursulin
  0 siblings, 0 replies; 32+ messages in thread
From: Tvrtko Ursulin @ 2022-02-25 18:35 UTC (permalink / raw)
  To: Michal Wajdeczko, John Harrison, Jordan Justen, intel-gfx
  Cc: dri-devel, Rodrigo Vivi


On 25/02/2022 18:05, Michal Wajdeczko wrote:
> On 25.02.2022 18:18, Tvrtko Ursulin wrote:
>>
>> On 25/02/2022 16:46, John Harrison wrote:
>>
>>>>> driver we don't care that much that we failed to load HWconfig and
>>>>> 'notice' is enough.
>>>>>
>>>>> but I'm fine with all messages being drm_err (as we will not have to
>>>>> change that once again after HWconfig will be mandatory for the driver
>>>>> as well)
>>>>
>>>> I would be against drm_err.
>>>>
>>>> #define KERN_EMERG      KERN_SOH "0"    /* system is unusable */
>>>> #define KERN_ALERT      KERN_SOH "1"    /* action must be taken
>>>> immediately */
>>>> #define KERN_CRIT       KERN_SOH "2"    /* critical conditions */
>>>> #define KERN_ERR        KERN_SOH "3"    /* error conditions */
>>>> #define KERN_WARNING    KERN_SOH "4"    /* warning conditions */
>>>> #define KERN_NOTICE     KERN_SOH "5"    /* normal but significant
>>>> condition */
>>>> #define KERN_INFO       KERN_SOH "6"    /* informational */
>>>> #define KERN_DEBUG      KERN_SOH "7"    /* debug-level messages */
>>>>
>>>>  From the point of view of the kernel driver, this is not an error to
>>>> its operation. It can at most be a warning, but notice is also fine
>>>> by me. One could argue when reading "normal but significant
>>>> condition" that it is not normal, when it is in fact unexpected, so
>>>> if people prefer warning that is also okay by me. I still lean
>>>> towards notice becuase of the hands-off nature i915 has with the
>>>> pass-through of this blob.
>>>   From the point of view of the KMD, i915 will load and be 'functional'
>>> if it can't talk to the hardware at all. The UMDs won't work at all but
>>
>> Well this reductio ad absurdum fails I think... :)
>>
>>> the driver load will be 'fine'. That's a requirement to be able to get
>>> the user to a software fallback desktop in order to work out why the
>>> hardware isn't working (e.g. no GuC firmware file). I would view this
>>> as similar. The KMD might have loaded but the UMDs are not functional.
>>> That is definitely an error condition to me.
>>
>> ... If GuC fails to load there is no command submission and driver will
>> obviously log that with drm_err.
>>
>> If blob fails to verify it depends on the userspace stack what will
>> happen. As stated before on some platforms, and/or after a certain time,
>> Mesa will not look at the blob at all. So i915 is fine (it is after all
>> just a conduit for opaque data!), system overall is fine, so it
>> definitely isn't a KERN_ERR level event.
>>
>>>>>>>> +               ERR_PTR(ret));
>>>>>>>> +
>>>>>>>>         ret = guc_enable_communication(guc);
>>>>>>>>         if (ret)
>>>>>>>>             goto err_log_capture;
>>>>>>>> @@ -562,6 +567,8 @@ static void __uc_fini_hw(struct intel_uc *uc)
>>>>>>>>         if (intel_uc_uses_guc_submission(uc))
>>>>>>>>             intel_guc_submission_disable(guc);
>>>>>>>>     +    intel_guc_hwconfig_fini(&guc->hwconfig);
>>>>>>>> +
>>>>>>>>         __uc_sanitize(uc);
>>>>>>>>     }
>>>>>>>>     diff --git a/drivers/gpu/drm/i915/i915_pci.c
>>>>>>>> b/drivers/gpu/drm/i915/i915_pci.c
>>>>>>>> index 76e590fcb903..1d31e35a5154 100644
>>>>>>>> --- a/drivers/gpu/drm/i915/i915_pci.c
>>>>>>>> +++ b/drivers/gpu/drm/i915/i915_pci.c
>>>>>>>> @@ -990,6 +990,7 @@ static const struct intel_device_info
>>>>>>>> adl_p_info = {
>>>>>>>>             BIT(RCS0) | BIT(BCS0) | BIT(VECS0) | BIT(VCS0) |
>>>>>>>> BIT(VCS2),
>>>>>>>>         .ppgtt_size = 48,
>>>>>>>>         .dma_mask_size = 39,
>>>>>>>> +    .has_guc_hwconfig = 1,
>>>>>>> Who requested this change? It was previously done this way but the
>>>>>>> instruction was that i915_pci.c is for hardware features only but
>>>>>>> that
>>>>>>> this, as you seem extremely keen about pointing out at every
>>>>>>> opportunity, is a software feature.
>>>>>>
>>>>>> This was requested by Michal as well. I definitely agree it is a
>>>>>> software feature, but I was not aware that "i915_pci.c is for hardware
>>>>>> features only".
>>>>>>
>>>>>> Michal, do you agree with this and returning to the previous method
>>>>>> for
>>>>>> enabling the feature?
>>>>>
>>>>> now I'm little confused as some arch direction was to treat FW as
>>>>> extension of the HW so for me it was natural to have 'has_guc_hwconfig'
>>>>> flag in device_info
>>>>>
>>>>> if still for some reason it is undesired to mix HW and FW/SW flags
>>>>> inside single group of flags then maybe we should just add separate
>>>>> group of immutable flags where has_guc_hwconfig could be defined.
>>>>>
>>>>> let our maintainers decide
>>>>
>>>> Bah.. :)
>>>>
>>>> And what was the previous method?
>>>>
>>>> [comes back later]
>>>>
>>>> Okay it was:
>>>>
>>>> +static bool has_table(struct drm_i915_private *i915)
>>>> +{
>>>> +    if (IS_ALDERLAKE_P(i915))
>>>> +        return true;
>>>>
>>>> Which sucks a bit if we want to argue it does not belong in device info.
>>>>
>>>> Why can't we ask the GuC if the blob exists? In fact what would
>>>> happen if one would call __guc_action_get_hwconfig on any GuC platform?
>>> That was how I originally wrote the code. However, other parties
>>> refuse to allow a H2G call to fail. The underlying CTB layers complain
>>> loudly on any CTB error. And the GuC architects insist that a call to
>>> query the table on an unsupported platform is an error and should
>>> return an 'unsupported' error code.
> 
> just for the background: IIRC the reason why arch didn't want 'query'
> mode that wont fail was that HWconfig is not optional on these selected
> platforms, so driver shall know up-front on which platforms it shall be
> working (and driver shall even fail if blob is not available)
> 
>>
>> Oh well, shrug, sounds silly but I will not pretend I am familiar with H2G
>>
>> In this case has_table does sound better since it indeed isn't a
>> hardware feature. It is a GuC fw thing and if we don't have a way to
>> probe things there at runtime, then at least limit the knowledge to GuC
>> files.
> 
> note that arch expectation is that driver itself shall be using this
> hwconfig (ie. will decode required data). while current approach is that
> driver acts only as a proxy to UMD, this has to change and refactoring
> will start (likely) soon. therefore flag has_guc_hwconfig fits better
> IMHO as this wont be just 'GuC' fw thing.

There we go, Jordan's validation gets a new unexpected use. :D

In this case it should maybe be like:

#define I915_NEEDS_GUC_HWCONFIG(i915) (IS_PLATFORM(xxx) || VER >= nnn)

Since it is still not a hardware feature like other device info flags.

Regards,

Tvrtko

^ permalink raw reply	[flat|nested] 32+ messages in thread
end of thread, other threads:[~2022-02-25 18:35 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-22 10:36 [PATCH v5 0/4] GuC HWCONFIG with documentation Jordan Justen
2022-02-22 10:36 ` [Intel-gfx] " Jordan Justen
2022-02-22 10:36 ` [PATCH v5 1/4] drm/i915/guc: Add fetch of hwconfig table Jordan Justen
2022-02-22 10:36   ` [Intel-gfx] " Jordan Justen
2022-02-24 13:58   ` Michal Wajdeczko
2022-02-24 13:58     ` [Intel-gfx] " Michal Wajdeczko
2022-02-25  2:17   ` John Harrison
2022-02-25  2:17     ` [Intel-gfx] " John Harrison
2022-02-25  2:24     ` [PATCH] " John.C.Harrison
2022-02-25  2:24       ` [Intel-gfx] " John.C.Harrison
2022-02-25  5:03     ` [PATCH v5 1/4] " Jordan Justen
2022-02-25  5:03       ` [Intel-gfx] " Jordan Justen
2022-02-25  9:44       ` Michal Wajdeczko
2022-02-25  9:44         ` [Intel-gfx] " Michal Wajdeczko
2022-02-25 13:26         ` Tvrtko Ursulin
2022-02-25 16:46           ` John Harrison
2022-02-25 17:18             ` Tvrtko Ursulin
2022-02-25 18:05               ` Michal Wajdeczko
2022-02-25 18:35                 ` Tvrtko Ursulin
2022-02-22 10:36 ` [PATCH v5 2/4] drm/i915/uapi: Add query for hwconfig blob Jordan Justen
2022-02-22 10:36   ` [Intel-gfx] " Jordan Justen
2022-02-22 10:36 ` [PATCH v5 3/4] drm/i915/uapi: Add struct drm_i915_query_hwconfig_blob_item Jordan Justen
2022-02-22 10:36   ` [Intel-gfx] " Jordan Justen
2022-02-22 10:36 ` [PATCH v5 4/4] drm/i915/guc: Verify hwconfig blob matches supported format Jordan Justen
2022-02-22 10:36   ` [Intel-gfx] " Jordan Justen
2022-02-22 11:24   ` Tvrtko Ursulin
2022-02-22 11:28 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for GuC HWCONFIG with documentation (rev5) Patchwork
2022-02-22 11:29 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2022-02-22 12:03 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2022-02-25  2:07   ` John Harrison
2022-02-22 13:30 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
2022-02-25  7:20 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for GuC HWCONFIG with documentation (rev6) Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.