Re: [PATCH 2/3] drm/i915/uc: Move uc firmware layout definitions to dedicated file

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 7/24/19 1:01 PM, Michal Wajdeczko wrote:
On Wed, 24 Jul 2019 19:50:37 +0200, Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx> wrote:



On 7/24/19 10:34 AM, Michal Wajdeczko wrote:
Generic uc firmware layout definitions are unlikely to change and
are separate to other GuC specific definitions.
 Signed-off-by: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>

Keeping things that apply to HuC as well in a generic file seems sensible to me.

---
  Documentation/gpu/i915.rst                   |  2 +-
  drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h  | 70 -----------------
  drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c     |  1 +
  drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h | 81 ++++++++++++++++++++
  4 files changed, 83 insertions(+), 71 deletions(-)
  create mode 100644 drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h
 diff --git a/Documentation/gpu/i915.rst b/Documentation/gpu/i915.rst
index c2173d120492..366cb7f46d17 100644
--- a/Documentation/gpu/i915.rst
+++ b/Documentation/gpu/i915.rst
@@ -448,7 +448,7 @@ GuC-based command submission
  GuC Firmware Layout
  -------------------
  -.. kernel-doc:: drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h
+.. kernel-doc:: drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h
     :doc: GuC Firmware Layout

This is now generic uC firmware layout

    GuC Address Space
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h
index 30cca3a29323..06a9bdfb0faf 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h
@@ -121,76 +121,6 @@
    #define GUC_CTL_MAX_DWORDS        (SOFT_SCRATCH_COUNT - 2) /* [1..14] */
  -/**
- * DOC: GuC Firmware Layout
- *
- * The GuC firmware layout looks like this:
- *
- *     +-------------------------------+
- *     |         uc_css_header         |
- *     |                               |
- *     | contains major/minor version  |
- *     +-------------------------------+
- *     |             uCode             |
- *     +-------------------------------+
- *     |         RSA signature         |
- *     +-------------------------------+
- *     |          modulus key          |
- *     +-------------------------------+
- *     |          exponent val         |
- *     +-------------------------------+
- *
- * The firmware may or may not have modulus key and exponent data. The header, - * uCode and RSA signature are must-have components that will be used by driver. - * Length of each components, which is all in dwords, can be found in header. - * In the case that modulus and exponent are not present in fw, a.k.a truncated
- * image, the length value still appears in header.
- *
- * Driver will do some basic fw size validation based on the following rules:
- *
- * 1. Header, uCode and RSA are must-have components.
- * 2. All firmware components, if they present, are in the sequence illustrated
- *    in the layout table above.
- * 3. Length info of each component can be found in header, in dwords.
- * 4. Modulus and exponent key are not required by driver. They may not appear
- *    in fw. So driver will load a truncated firmware in this case.
- *
- * HuC firmware layout is same as GuC firmware.
- * Only HuC version information is saved in a different way.
- */
-
-struct uc_css_header {
-    u32 module_type;
-    /* header_size includes all non-uCode bits, including css_header, rsa
-     * key, modulus key and exponent data. */
-    u32 header_size_dw;
-    u32 header_version;
-    u32 module_id;
-    u32 module_vendor;
-    u32 date;
-#define CSS_DATE_DAY            (0xFF << 0)
-#define CSS_DATE_MONTH            (0xFF << 8)
-#define CSS_DATE_YEAR            (0xFFFF << 16)
-    u32 size_dw; /* uCode plus header_size_dw */
-    u32 key_size_dw;
-    u32 modulus_size_dw;
-    u32 exponent_size_dw;
-    u32 time;
-#define CSS_TIME_HOUR            (0xFF << 0)
-#define CSS_DATE_MIN            (0xFF << 8)
-#define CSS_DATE_SEC            (0xFFFF << 16)
-    char username[8];
-    char buildnumber[12];
-    u32 sw_version;
-#define CSS_SW_VERSION_GUC_MAJOR    (0xFF << 16)
-#define CSS_SW_VERSION_GUC_MINOR    (0xFF << 8)
-#define CSS_SW_VERSION_GUC_PATCH    (0xFF << 0)
-#define CSS_SW_VERSION_HUC_MAJOR    (0xFFFF << 16)
-#define CSS_SW_VERSION_HUC_MINOR    (0xFFFF << 0)
-    u32 reserved[14];
-    u32 header_info;
-} __packed;
-
  /* Work item for submitting workloads into work queue of GuC. */
  struct guc_wq_item {
      u32 header;
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
index 8ce7210907c0..d5cb19b4e5c1 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
@@ -27,6 +27,7 @@
  #include <drm/drm_print.h>
    #include "intel_uc_fw.h"
+#include "intel_uc_fw_abi.h"
  #include "i915_drv.h"
    /**
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h
new file mode 100644
index 000000000000..3ca535534151
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h
@@ -0,0 +1,81 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2019 Intel Corporation
+ */
+
+#ifndef _INTEL_UC_FW_ABI_H
+#define _INTEL_UC_FW_ABI_H
+
+#include <linux/types.h>
+
+/**
+ * DOC: GuC Firmware Layout
+ *
+ * The GuC firmware layout looks like this:

same here, s/GuC/uC.

+ *
+ *     +-------------------------------+
+ *     |         uc_css_header         |
+ *     |                               |
+ *     | contains major/minor version  |
+ *     +-------------------------------+
+ *     |             uCode             |
+ *     +-------------------------------+
+ *     |         RSA signature         |
+ *     +-------------------------------+
+ *     |          modulus key          |
+ *     +-------------------------------+
+ *     |          exponent val         |
+ *     +-------------------------------+
+ *
+ * The firmware may or may not have modulus key and exponent data. The header, + * uCode and RSA signature are must-have components that will be used by driver. + * Length of each components, which is all in dwords, can be found in header. + * In the case that modulus and exponent are not present in fw, a.k.a truncated
+ * image, the length value still appears in header.
+ *
+ * Driver will do some basic fw size validation based on the following rules:
+ *
+ * 1. Header, uCode and RSA are must-have components.
+ * 2. All firmware components, if they present, are in the sequence illustrated
+ *    in the layout table above.
+ * 3. Length info of each component can be found in header, in dwords.
+ * 4. Modulus and exponent key are not required by driver. They may not appear
+ *    in fw. So driver will load a truncated firmware in this case.
+ *
+ * HuC firmware layout is same as GuC firmware.
+ * Only HuC version information is saved in a different way.

And reword this as something like: "The only difference between GuC and HuC firmwares is how the version information is saved"

With that:

As aim of this patch was just to move definition as-is from one place
to the other, can I make all above fixups in other patch ?

Note that changing "DOC" tag will require corresponding changes in .rst
and I don't want to that in here

Having the changes in a new patch in the series works for me. I'd prefer to have them before the move but not a blocker.

Daniele



Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>

Daniele

+ */
+
+struct uc_css_header {
+    u32 module_type;
+    /* header_size includes all non-uCode bits, including css_header, rsa
+     * key, modulus key and exponent data. */
+    u32 header_size_dw;
+    u32 header_version;
+    u32 module_id;
+    u32 module_vendor;
+    u32 date;
+#define CSS_DATE_DAY            (0xFF << 0)
+#define CSS_DATE_MONTH            (0xFF << 8)
+#define CSS_DATE_YEAR            (0xFFFF << 16)
+    u32 size_dw; /* uCode plus header_size_dw */
+    u32 key_size_dw;
+    u32 modulus_size_dw;
+    u32 exponent_size_dw;
+    u32 time;
+#define CSS_TIME_HOUR            (0xFF << 0)
+#define CSS_DATE_MIN            (0xFF << 8)
+#define CSS_DATE_SEC            (0xFFFF << 16)
+    char username[8];
+    char buildnumber[12];
+    u32 sw_version;
+#define CSS_SW_VERSION_GUC_MAJOR    (0xFF << 16)
+#define CSS_SW_VERSION_GUC_MINOR    (0xFF << 8)
+#define CSS_SW_VERSION_GUC_PATCH    (0xFF << 0)
+#define CSS_SW_VERSION_HUC_MAJOR    (0xFFFF << 16)
+#define CSS_SW_VERSION_HUC_MINOR    (0xFFFF << 0)
+    u32 reserved[14];
+    u32 header_info;
+} __packed;
+
+#endif /* _INTEL_UC_FW_ABI_H */
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux