[Intel-gfx] [PATCH v2 2/3] drm/i915/guc: improve documentation

Thu Oct 10 07:48:53 UTC 2019

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 at intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
> Cc: Matthew Brost <matthew.brost at intel.com>
> Cc: Martin Peres <martin.peres at linux.intel.com>
> Acked-by: Anna Karas <anna.karas at 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 at 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 {
> 