On 28/09/2019 00:42, 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.
>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
> Cc: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
> ---
> Documentation/gpu/i915.rst | 10 ++++++++--
> drivers/gpu/drm/i915/gt/uc/intel_huc.c | 19 +++++++++++++++----
> drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c | 15 ---------------
> 3 files changed, 23 insertions(+), 21 deletions(-)
>
> diff --git a/Documentation/gpu/i915.rst b/Documentation/gpu/i915.rst
> index 357e9dfa7de1..bfb64337db66 100644
> --- a/Documentation/gpu/i915.rst
> +++ b/Documentation/gpu/i915.rst
> @@ -471,8 +471,14 @@ 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 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 d4625c97b4f9..6e10fe898c90 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> @@ -9,6 +9,18 @@
> #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.
> + */
> +
> void intel_huc_init_early(struct intel_huc *huc)
> {
> struct drm_i915_private *i915 = huc_to_gt(huc)->i915;
> @@ -118,10 +130,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.
Having a link to the media driver here that would explain what the HuC
enables would be beneficial (we don't want to maintain that).
> - *
> - * 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.
> - */
Could we add a section explaining the access the HuC has to memory, and
one stating that the firmware is optional?
Anyways, thanks! The series could be landed as-is already, but a few
more additions would be welcomed :)
Martin
> -
> /**
> * intel_huc_fw_init_early() - initializes HuC firmware struct
> * @huc: intel_huc struct
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
