[Intel-gfx] [PATCH 3/3] drm/i915/huc: Update HuC status codes

[Joonas Lahtinen]

Quoting Michal Wajdeczko (2019-05-23 01:25:00)
> On Wed, 22 May 2019 22:19:47 +0200, Chris Wilson  
> <chris at chris-wilson.co.uk> wrote:
> 
> > Quoting Michal Wajdeczko (2019-05-22 20:00:57)
> >> Without breaking existing usage, slightly update HuC status codes
> >> to provide more info to the clients:
> >>  1 if HuC firmware is loaded and verified,
> >>  0 if HuC firmware is not enabled,
> >>  -ENOPKG if HuC firmware is not loaded,
> >>  -ENODEV if HuC is not present on this platform.
> >>
> >> Signed-off-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
> >> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> >> Cc: Tony Ye <tony.ye at intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/intel_huc.c | 13 +++++++++----
> >>  1 file changed, 9 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_huc.c  
> >> b/drivers/gpu/drm/i915/intel_huc.c
> >> index aac17916e130..98deb4ee60a7 100644
> >> --- a/drivers/gpu/drm/i915/intel_huc.c
> >> +++ b/drivers/gpu/drm/i915/intel_huc.c
> >> @@ -150,9 +150,11 @@ int intel_huc_auth(struct intel_huc *huc)
> >>   * intel_huc_check_status() - check HuC status
> >>   * @huc: intel_huc structure
> >>   *
> >> - * Returns: 1 if HuC firmware is loaded and verified,
> >> - * 0 if HuC firmware is not loaded and -ENODEV if HuC
> >> - * is not present on this platform.
> >> + * Return:
> >> + * * 1 if HuC firmware is loaded and verified,
> >> + * * 0 if HuC firmware is not enabled,
> >> + * * -ENOPKG if HuC firmware is not loaded,
> >> + * * -ENODEV if HuC is not present on this platform.
> >>   */
> >>  int intel_huc_check_status(struct intel_huc *huc)
> >>  {
> >> @@ -161,5 +163,8 @@ int intel_huc_check_status(struct intel_huc *huc)
> >>         if (!HAS_HUC(i915))
> >>                 return -ENODEV;
> >>
> >> -       return huc->verified;
> >> +       if (!USES_HUC(i915))
> >> +               return 0;
> >> +
> >> +       return huc->verified ? 1 : -ENOPKG;
> >
> > I still think EOPNOTSUPP is a better error though for the user
> > preventing the huc being loaded -- as opposed to the result of
> > verification being the non-error value.
> >
> > error == unable to setup huc
> > 0/1 == result from talking to huc
> 
> but your 0 here overlaps with unable to setup huc error,
> so from the ABI perspective, imho, is bad.
> 
> also, from media team pov, as they want to have HuC always on,
> the only non-error case is when user explicitly decided otherwise.

Trying to look things from external perspective, if HUC_(AUTHENTICATION_)STATUS
is queried 1 => "authenticated", 0 => "not authenticated" makes most sense.

Both media drivers also first check for errors, then convert the return
value to boolean, so it would be compatible with that.

Regards, Joonas