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

Message ID 20191010010214.11591-3-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
Better explain the usage of the microcontroller and what i915 is
responsible of. While at it, fix the documentation for the auth
function, which doesn't do any pinning anymore.

v2: add a comment on HuC being optional and descrive how HuC accesses
    memory (Martin)

Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Martin Peres <martin.peres@linux.intel.com>
Acked-by: Anna Karas <anna.karas@intel.com>
---
 Documentation/gpu/i915.rst                | 16 +++++++++--
 drivers/gpu/drm/i915/gt/uc/intel_huc.c    | 33 ++++++++++++++++++++---
 drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c | 15 -----------
 3 files changed, 43 insertions(+), 21 deletions(-)

Comments

Martin Peres Oct. 10, 2019, 10:09 a.m. UTC | #1
On 10/10/2019 04:02, Daniele Ceraolo Spurio wrote:
> Better explain the usage of the microcontroller and what i915 is
> responsible of. While at it, fix the documentation for the auth
> function, which doesn't do any pinning anymore.
> 
> v2: add a comment on HuC being optional and descrive how HuC accesses
>     memory (Martin)
> 
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Martin Peres <martin.peres@linux.intel.com>
> Acked-by: Anna Karas <anna.karas@intel.com>
> ---
>  Documentation/gpu/i915.rst                | 16 +++++++++--
>  drivers/gpu/drm/i915/gt/uc/intel_huc.c    | 33 ++++++++++++++++++++---
>  drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c | 15 -----------
>  3 files changed, 43 insertions(+), 21 deletions(-)
> 
> diff --git a/Documentation/gpu/i915.rst b/Documentation/gpu/i915.rst
> index 357e9dfa7de1..60bd6e6403da 100644
> --- a/Documentation/gpu/i915.rst
> +++ b/Documentation/gpu/i915.rst
> @@ -471,8 +471,20 @@ GuC-based command submission
>  
>  HuC
>  ---
> -.. kernel-doc:: drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
> -   :doc: HuC Firmware
> +.. kernel-doc:: drivers/gpu/drm/i915/gt/uc/intel_huc.c
> +   :doc: HuC
> +.. kernel-doc:: drivers/gpu/drm/i915/gt/uc/intel_huc.c
> +   :functions: intel_huc_auth
> +
> +HuC Memory Management
> +~~~~~~~~~~~~~~~~~~~~~
> +
> +.. kernel-doc:: drivers/gpu/drm/i915/gt/uc/intel_huc.c
> +   :doc: HuC Memory Management
> +
> +HuC Firmware Layout
> +~~~~~~~~~~~~~~~~~~~
> +The HuC FW layout is the same as the GuC one, see `GuC Firmware Layout`_
>  
>  DMC
>  ---
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.c b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> index 33608a114d4e..c802e5b68c05 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> @@ -9,6 +9,32 @@
>  #include "intel_huc.h"
>  #include "i915_drv.h"
>  
> +/**
> + * DOC: HuC
> + *
> + * The HuC is a dedicated microcontroller for usage in media HEVC (High
> + * Efficiency Video Coding) operations. Userspace can directly use the firmware
> + * capabilities by adding HuC specific commands to batch buffers.

Would be good to separate paragraphs with an empty line, for consistency.

> + * The kernel driver is only responsible for loading the HuC firmware and
> + * triggering its security authentication, which is performed by the GuC. For
> + * The GuC to correctly perform the authentication, the HuC binary must be
> + * loaded before the GuC one. Loading the HuC is optional; however, not using
> + * the HuC might negatively impact power usage and/or performance of media
> + * workloads, depending on the use-cases.

Same here.

> + * See https://github.com/intel/media-driver for the latest details on HuC
> + * functionality.

Well, isn't that the perfect link!? The media team has done a great job
on their communication!

> + */
> +
> +/**
> + * DOC: HuC Memory Management
> + *
> + * Similarly to the GuC, the HuC can't do any memory allocations on its own,
> + * with the difference being that the allocations for HuC usage are handled by
> + * the userspace driver instead of the kernel one. The HuC accesses the memory
> + * via the PPGTT belonging to the context loaded on the VCS executing the
> + * HuC-specific commands.
> + */

Thanks!

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

Now, only one firmware is left to document! Jani, can you take actions
to improve the DMC documentation?

Martin

> +
>  void intel_huc_init_early(struct intel_huc *huc)
>  {
>  	struct drm_i915_private *i915 = huc_to_gt(huc)->i915;
> @@ -118,10 +144,9 @@ void intel_huc_fini(struct intel_huc *huc)
>   *
>   * Called after HuC and GuC firmware loading during intel_uc_init_hw().
>   *
> - * This function pins HuC firmware image object into GGTT.
> - * Then it invokes GuC action to authenticate passing the offset to RSA
> - * signature through intel_guc_auth_huc(). It then waits for 50ms for
> - * firmware verification ACK and unpins the object.
> + * This function invokes the GuC action to authenticate the HuC firmware,
> + * passing the offset of the RSA signature to intel_guc_auth_huc(). It then
> + * waits for up to 50ms for firmware verification ACK.
>   */
>  int intel_huc_auth(struct intel_huc *huc)
>  {
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
> index 74602487ed67..d654340d4d03 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
> @@ -7,21 +7,6 @@
>  #include "intel_huc_fw.h"
>  #include "i915_drv.h"
>  
> -/**
> - * DOC: HuC Firmware
> - *
> - * Motivation:
> - * GEN9 introduces a new dedicated firmware for usage in media HEVC (High
> - * Efficiency Video Coding) operations. Userspace can use the firmware
> - * capabilities by adding HuC specific commands to batch buffers.
> - *
> - * Implementation:
> - * The same firmware loader is used as the GuC. However, the actual
> - * loading to HW is deferred until GEM initialization is done.
> - *
> - * Note that HuC firmware loading must be done before GuC loading.
> - */
> -
>  /**
>   * intel_huc_fw_init_early() - initializes HuC firmware struct
>   * @huc: intel_huc struct
>

Patch
diff mbox series

diff --git a/Documentation/gpu/i915.rst b/Documentation/gpu/i915.rst
index 357e9dfa7de1..60bd6e6403da 100644
--- a/Documentation/gpu/i915.rst
+++ b/Documentation/gpu/i915.rst
@@ -471,8 +471,20 @@  GuC-based command submission
 
 HuC
 ---
-.. kernel-doc:: drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
-   :doc: HuC Firmware
+.. kernel-doc:: drivers/gpu/drm/i915/gt/uc/intel_huc.c
+   :doc: HuC
+.. kernel-doc:: drivers/gpu/drm/i915/gt/uc/intel_huc.c
+   :functions: intel_huc_auth
+
+HuC Memory Management
+~~~~~~~~~~~~~~~~~~~~~
+
+.. kernel-doc:: drivers/gpu/drm/i915/gt/uc/intel_huc.c
+   :doc: HuC Memory Management
+
+HuC Firmware Layout
+~~~~~~~~~~~~~~~~~~~
+The HuC FW layout is the same as the GuC one, see `GuC Firmware Layout`_
 
 DMC
 ---
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.c b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
index 33608a114d4e..c802e5b68c05 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
@@ -9,6 +9,32 @@ 
 #include "intel_huc.h"
 #include "i915_drv.h"
 
+/**
+ * DOC: HuC
+ *
+ * The HuC is a dedicated microcontroller for usage in media HEVC (High
+ * Efficiency Video Coding) operations. Userspace can directly use the firmware
+ * capabilities by adding HuC specific commands to batch buffers.
+ * The kernel driver is only responsible for loading the HuC firmware and
+ * triggering its security authentication, which is performed by the GuC. For
+ * The GuC to correctly perform the authentication, the HuC binary must be
+ * loaded before the GuC one. Loading the HuC is optional; however, not using
+ * the HuC might negatively impact power usage and/or performance of media
+ * workloads, depending on the use-cases.
+ * See https://github.com/intel/media-driver for the latest details on HuC
+ * functionality.
+ */
+
+/**
+ * DOC: HuC Memory Management
+ *
+ * Similarly to the GuC, the HuC can't do any memory allocations on its own,
+ * with the difference being that the allocations for HuC usage are handled by
+ * the userspace driver instead of the kernel one. The HuC accesses the memory
+ * via the PPGTT belonging to the context loaded on the VCS executing the
+ * HuC-specific commands.
+ */
+
 void intel_huc_init_early(struct intel_huc *huc)
 {
 	struct drm_i915_private *i915 = huc_to_gt(huc)->i915;
@@ -118,10 +144,9 @@  void intel_huc_fini(struct intel_huc *huc)
  *
  * Called after HuC and GuC firmware loading during intel_uc_init_hw().
  *
- * This function pins HuC firmware image object into GGTT.
- * Then it invokes GuC action to authenticate passing the offset to RSA
- * signature through intel_guc_auth_huc(). It then waits for 50ms for
- * firmware verification ACK and unpins the object.
+ * This function invokes the GuC action to authenticate the HuC firmware,
+ * passing the offset of the RSA signature to intel_guc_auth_huc(). It then
+ * waits for up to 50ms for firmware verification ACK.
  */
 int intel_huc_auth(struct intel_huc *huc)
 {
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
index 74602487ed67..d654340d4d03 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
@@ -7,21 +7,6 @@ 
 #include "intel_huc_fw.h"
 #include "i915_drv.h"
 
-/**
- * DOC: HuC Firmware
- *
- * Motivation:
- * GEN9 introduces a new dedicated firmware for usage in media HEVC (High
- * Efficiency Video Coding) operations. Userspace can use the firmware
- * capabilities by adding HuC specific commands to batch buffers.
- *
- * Implementation:
- * The same firmware loader is used as the GuC. However, the actual
- * loading to HW is deferred until GEM initialization is done.
- *
- * Note that HuC firmware loading must be done before GuC loading.
- */
-
 /**
  * intel_huc_fw_init_early() - initializes HuC firmware struct
  * @huc: intel_huc struct