| Message ID | 20190927214243.18558-3-daniele.ceraolospurio@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/3] drm/i915: Add microcontrollers documentation section | expand |
Hello Daniele, On Fri, Sep 27, 2019 at 02:42:43PM -0700, 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@intel.com> > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> Acked-by: Anna Karas <anna.karas@intel.com> Anna Karas
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@intel.com> > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> > --- > 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 >
On 10/9/19 7:41 AM, Martin Peres wrote: > 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@intel.com> >> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> >> --- >> 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 I'll have to follow up with the HuC team to understand how the memory access works because I'm not too familiar with HuC. Are you ok if I address all your other comments for GuC and HuC and add this as a follow up later once I get the info? Daniele > 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 >>
On 10/9/19 2:44 PM, Daniele Ceraolo Spurio wrote: > > > On 10/9/19 7:41 AM, Martin Peres wrote: >> 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@intel.com> >>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> >>> --- >>> 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 > > I'll have to follow up with the HuC team to understand how the memory > access works because I'm not too familiar with HuC. Are you ok if I > address all your other comments for GuC and HuC and add this as a follow > up later once I get the info? > Never mind, I found the info I needed (Bspec 48058), so I can do all the changes in one go. Daniele > Daniele > >> 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@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
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. - * - * 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
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@intel.com> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> --- 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(-)