Quoting Michal Wajdeczko (2019-05-23 01:25:00)
> On Wed, 22 May 2019 22:19:47 +0200, Chris Wilson
> <chris@xxxxxxxxxxxxxxxxxx> 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@xxxxxxxxx>
> >> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> >> Cc: Tony Ye <tony.ye@xxxxxxxxx>
> >> ---
> >> 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
