Re: [Intel-gfx] [PATCH v2 2/3] drm/i915/guc: improve documentation
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
> Cc: Michal Wajdeczko
> Cc: Matthew Brost
> Cc: Martin Peres
> Acked-by: Anna Karas
> ---
> 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
> + * 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
[Intel-gfx] [PATCH v2 2/3] drm/i915/guc: improve documentation
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
Cc: Michal Wajdeczko
Cc: Matthew Brost
Cc: Martin Peres
Acked-by: Anna Karas
---
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.
+ * 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
