[v2,2/3] drm/i915/guc: improve documentation
diff mbox series

Message ID 20191010010214.11591-2-daniele.ceraolospurio@intel.com
State New
Headers show
Series
  • [v2,1/3] drm/i915: Add microcontrollers documentation section
Related show

Commit Message

Daniele Ceraolo Spurio Oct. 10, 2019, 1:02 a.m. UTC
Add a short description of what we expect from GuC and some minor
improvements to existing documentation. Also remove a comment about a
difference between GuC and HuC that is not true anymore.

v2: add that the GuC is not mandatory (Martin)

Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Matthew Brost <matthew.brost@intel.com>
Cc: Martin Peres <martin.peres@linux.intel.com>
Acked-by: Anna Karas <anna.karas@intel.com>
---
 Documentation/gpu/i915.rst                    | 22 +++++++++-----
 drivers/gpu/drm/i915/gt/uc/intel_guc.c        | 30 +++++++++++++++++--
 .../gpu/drm/i915/gt/uc/intel_guc_submission.c |  6 ++++
 drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h  |  3 --
 4 files changed, 48 insertions(+), 13 deletions(-)

Comments

Martin Peres Oct. 10, 2019, 7:48 a.m. UTC | #1
On 10/10/2019 04:02, Daniele Ceraolo Spurio wrote:
> Add a short description of what we expect from GuC and some minor
> improvements to existing documentation. Also remove a comment about a
> difference between GuC and HuC that is not true anymore.
> 
> v2: add that the GuC is not mandatory (Martin)
> 
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Matthew Brost <matthew.brost@intel.com>
> Cc: Martin Peres <martin.peres@linux.intel.com>
> Acked-by: Anna Karas <anna.karas@intel.com>
> ---
>  Documentation/gpu/i915.rst                    | 22 +++++++++-----
>  drivers/gpu/drm/i915/gt/uc/intel_guc.c        | 30 +++++++++++++++++--
>  .../gpu/drm/i915/gt/uc/intel_guc_submission.c |  6 ++++
>  drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h  |  3 --
>  4 files changed, 48 insertions(+), 13 deletions(-)
> 
> diff --git a/Documentation/gpu/i915.rst b/Documentation/gpu/i915.rst
> index f1bae7867045..357e9dfa7de1 100644
> --- a/Documentation/gpu/i915.rst
> +++ b/Documentation/gpu/i915.rst
> @@ -436,12 +436,24 @@ WOPCM Layout
>  GuC
>  ---
>  
> -Firmware Layout
> -~~~~~~~~~~~~~~~
> +.. kernel-doc:: drivers/gpu/drm/i915/gt/uc/intel_guc.c
> +   :doc: GuC
> +
> +GuC Firmware Layout
> +~~~~~~~~~~~~~~~~~~~
>  
>  .. kernel-doc:: drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h
>     :doc: Firmware Layout
>  
> +GuC Memory Management
> +~~~~~~~~~~~~~~~~~~~~~
> +
> +.. kernel-doc:: drivers/gpu/drm/i915/gt/uc/intel_guc.c
> +   :doc: GuC Memory Management
> +.. kernel-doc:: drivers/gpu/drm/i915/gt/uc/intel_guc.c
> +   :functions: intel_guc_allocate_vma
> +
> +
>  GuC-specific firmware loader
>  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>  
> @@ -457,12 +469,6 @@ GuC-based command submission
>  .. kernel-doc:: drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
>     :internal:
>  
> -GuC Address Space
> -~~~~~~~~~~~~~~~~~
> -
> -.. kernel-doc:: drivers/gpu/drm/i915/gt/uc/intel_guc.c
> -   :doc: GuC Address Space
> -
>  HuC
>  ---
>  .. kernel-doc:: drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> index 249c747e9756..ce97600790c2 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> @@ -9,6 +9,26 @@
>  #include "intel_guc_submission.h"
>  #include "i915_drv.h"
>  
> +/**
> + * DOC: GuC
> + *
> + * The GuC is a microcontroller inside the GT HW, introduced in gen9. The GuC is
> + * designed to offload some of the functionality usually performed by the host
> + * driver; currently the main operations it can take care of are:
> + *
> + * - Authentication of the HuC, which is required to fully enable HuC usage.
> + * - Low latency graphics context scheduling (a.k.a. GuC submission).
> + * - GT Power management.
> + *
> + * The enable_guc module parameter can be used to select which of those
> + * operations to enable within GuC. Note that not all the operations are
> + * supported on all gen9+ platforms.

Thanks for the changes!

With an added new line here, this patch is:

Reviewed-by: Martin Peres <martin.peres@linux.intel.com>

> + * Enabling the GuC is not mandatory and therefore the firmware is only loaded
> + * if at least one of the operations is selected. However, not loading the GuC
> + * might result in the loss of some features that do require the GuC (currently
> + * just the HuC, but more are expected to land in the future).
> + */
> +
>  static void gen8_guc_raise_irq(struct intel_guc *guc)
>  {
>  	struct intel_gt *gt = guc_to_gt(guc);
> @@ -548,9 +568,15 @@ int intel_guc_resume(struct intel_guc *guc)
>  }
>  
>  /**
> - * DOC: GuC Address Space
> + * DOC: GuC Memory Management
>   *
> - * The layout of GuC address space is shown below:
> + * GuC can't allocate any memory for its own usage, so all the allocations must
> + * be handled by the host driver. GuC accesses the memory via the GGTT, with the
> + * exception of the top and bottom parts of the 4GB address space, which are
> + * instead re-mapped by the GuC HW to memory location of the FW itself (WOPCM)
> + * or other parts of the HW. The driver must take care not to place objects that
> + * the GuC is going to access in these reserved ranges. The layout of the GuC
> + * address space is shown below:
>   *
>   * ::
>   *
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> index f325d3dd564f..849a44add424 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> @@ -29,6 +29,12 @@ enum {
>  /**
>   * DOC: GuC-based command submission
>   *
> + * IMPORTANT NOTE: GuC submission is currently not supported in i915. The GuC
> + * firmware is moving to an updated submission interface and we plan to
> + * turn submission back on when that lands. The below documentation (and related
> + * code) matches the old submission model and will be updated as part of the
> + * upgrade to the new flow.
> + *
>   * GuC client:
>   * A intel_guc_client refers to a submission path through GuC. Currently, there
>   * is only one client, which is charged with all submissions to the GuC. This
> 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
> index f8f6c91a0df6..029214cdedd5 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h
> @@ -39,9 +39,6 @@
>   * 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.
> - *
> - * The only difference between GuC and HuC firmwares is how the version
> - * information is saved.
>   */
>  
>  struct uc_css_header {
>

Patch
diff mbox series

diff --git a/Documentation/gpu/i915.rst b/Documentation/gpu/i915.rst
index f1bae7867045..357e9dfa7de1 100644
--- a/Documentation/gpu/i915.rst
+++ b/Documentation/gpu/i915.rst
@@ -436,12 +436,24 @@  WOPCM Layout
 GuC
 ---
 
-Firmware Layout
-~~~~~~~~~~~~~~~
+.. kernel-doc:: drivers/gpu/drm/i915/gt/uc/intel_guc.c
+   :doc: GuC
+
+GuC Firmware Layout
+~~~~~~~~~~~~~~~~~~~
 
 .. kernel-doc:: drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h
    :doc: Firmware Layout
 
+GuC Memory Management
+~~~~~~~~~~~~~~~~~~~~~
+
+.. kernel-doc:: drivers/gpu/drm/i915/gt/uc/intel_guc.c
+   :doc: GuC Memory Management
+.. kernel-doc:: drivers/gpu/drm/i915/gt/uc/intel_guc.c
+   :functions: intel_guc_allocate_vma
+
+
 GuC-specific firmware loader
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
@@ -457,12 +469,6 @@  GuC-based command submission
 .. kernel-doc:: drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
    :internal:
 
-GuC Address Space
-~~~~~~~~~~~~~~~~~
-
-.. kernel-doc:: drivers/gpu/drm/i915/gt/uc/intel_guc.c
-   :doc: GuC Address Space
-
 HuC
 ---
 .. kernel-doc:: drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
index 249c747e9756..ce97600790c2 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
@@ -9,6 +9,26 @@ 
 #include "intel_guc_submission.h"
 #include "i915_drv.h"
 
+/**
+ * DOC: GuC
+ *
+ * The GuC is a microcontroller inside the GT HW, introduced in gen9. The GuC is
+ * designed to offload some of the functionality usually performed by the host
+ * driver; currently the main operations it can take care of are:
+ *
+ * - Authentication of the HuC, which is required to fully enable HuC usage.
+ * - Low latency graphics context scheduling (a.k.a. GuC submission).
+ * - GT Power management.
+ *
+ * The enable_guc module parameter can be used to select which of those
+ * operations to enable within GuC. Note that not all the operations are
+ * supported on all gen9+ platforms.
+ * Enabling the GuC is not mandatory and therefore the firmware is only loaded
+ * if at least one of the operations is selected. However, not loading the GuC
+ * might result in the loss of some features that do require the GuC (currently
+ * just the HuC, but more are expected to land in the future).
+ */
+
 static void gen8_guc_raise_irq(struct intel_guc *guc)
 {
 	struct intel_gt *gt = guc_to_gt(guc);
@@ -548,9 +568,15 @@  int intel_guc_resume(struct intel_guc *guc)
 }
 
 /**
- * DOC: GuC Address Space
+ * DOC: GuC Memory Management
  *
- * The layout of GuC address space is shown below:
+ * GuC can't allocate any memory for its own usage, so all the allocations must
+ * be handled by the host driver. GuC accesses the memory via the GGTT, with the
+ * exception of the top and bottom parts of the 4GB address space, which are
+ * instead re-mapped by the GuC HW to memory location of the FW itself (WOPCM)
+ * or other parts of the HW. The driver must take care not to place objects that
+ * the GuC is going to access in these reserved ranges. The layout of the GuC
+ * address space is shown below:
  *
  * ::
  *
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index f325d3dd564f..849a44add424 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -29,6 +29,12 @@  enum {
 /**
  * DOC: GuC-based command submission
  *
+ * IMPORTANT NOTE: GuC submission is currently not supported in i915. The GuC
+ * firmware is moving to an updated submission interface and we plan to
+ * turn submission back on when that lands. The below documentation (and related
+ * code) matches the old submission model and will be updated as part of the
+ * upgrade to the new flow.
+ *
  * GuC client:
  * A intel_guc_client refers to a submission path through GuC. Currently, there
  * is only one client, which is charged with all submissions to the GuC. This
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
index f8f6c91a0df6..029214cdedd5 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h
@@ -39,9 +39,6 @@ 
  * 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.
- *
- * The only difference between GuC and HuC firmwares is how the version
- * information is saved.
  */
 
 struct uc_css_header {