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@xxxxxxxxx> > Cc: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx> > Cc: Martin Peres <martin.peres@xxxxxxxxxxxxxxx> > Acked-by: Anna Karas <anna.karas@xxxxxxxxx> > --- > 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@xxxxxxxxxxxxxxx> 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 > _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
