[Intel-gfx] [PATCH 8/8] drm/i915/get_params: Add HuC status to getparams
Arkadiusz Hiler
arkadiusz.hiler at intel.com
Mon Dec 12 14:52:05 UTC 2016
On Mon, Dec 12, 2016 at 02:21:41PM +0000, Chris Wilson wrote:
> On Mon, Dec 12, 2016 at 03:13:17PM +0100, Arkadiusz Hiler wrote:
> > On Fri, Dec 09, 2016 at 01:59:45PM +0100, Michal Wajdeczko wrote:
> > > On Thu, Dec 08, 2016 at 03:02:19PM -0800, anushasr wrote:
> > > > From: Peter Antoine <peter.antoine at intel.com>
> > > >
> > > > This patch will allow for getparams to return the status of the HuC.
> > > > As the HuC has to be validated by the GuC this patch uses the validated
> > > > status to show when the HuC is loaded and ready for use. You cannot use
> > > > the loaded status as with the GuC as the HuC is verified after it is
> > > > loaded and is not usable until it is verified.
> > > >
> > > > v2: removed the forewakes as the registers are already force-woken.
> > > > (T.Ursulin)
> > > > v4: rebased.
> > > > v5: rebased on top of drm-tip.
> > > > v6: rebased. Removed any reference to intel_huc.h
> > > >
> > > > Signed-off-by: Peter Antoine <peter.antoine at intel.com>
> > > > Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler at intel.com>
> > > > ---
> > > > drivers/gpu/drm/i915/i915_drv.c | 4 ++++
> > > > drivers/gpu/drm/i915/intel_huc_loader.c | 12 ++++++++++++
> > > > drivers/gpu/drm/i915/intel_uc.h | 1 +
> > > > include/uapi/drm/i915_drm.h | 1 +
> > > > 4 files changed, 18 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > > > index 85a47c2..6be06a27 100644
> > > > --- a/drivers/gpu/drm/i915/i915_drv.c
> > > > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > > > @@ -49,6 +49,7 @@
> > > > #include "i915_trace.h"
> > > > #include "i915_vgpu.h"
> > > > #include "intel_drv.h"
> > > > +#include "intel_uc.h"
> > > >
> > > > static struct drm_driver driver;
> > > >
> > > > @@ -349,6 +350,9 @@ static int i915_getparam(struct drm_device *dev, void *data,
> > > > */
> > > > value = 1;
> > > > break;
> > > > + case I915_PARAM_HAS_HUC:
> > >
> > > For me this param name does not match returned result which maybe misleading.
> > > Note that other HAS params return static driver/hw capability, not runtime.
> > >
> > > I guess PARAM_HUC_STATUS would be better (0=no huc, 1=pending, 2=ok, -1=failed)
> > > And you can cache huc status in intel_huc struct and make final modification in
> > > intel_huc_auth() function to avoid registry read (unless we want to detect later
> > > crash of the huc using this reg read)
> >
> > Why should userspace care for those intermediary states? From what I
> > know (docs and patches from the cover letter) userspace is interested
> > only in being able to use HuC. If something is not working you have
> > DebugFS for that exact purpose.
> >
> > As of cacheing - seems like a good idea to limit reg reads.
>
> Is GETPARAM(HAS_HUC) a hot path? Should we even encourage it?
Actually... Good point. HAS_HUC is used once each time you initialise
libva client.
> Are there any other users of intel_huc_is_valid()?
intel_guc_auth_huc() uses
ret = wait_for((I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED) > 0, 50);
So it can be reused there.
> As for userspace simply asking where huc is enabled, we already have
> that in the ABI via the module parameter, so you need to justify why
> this is preferred (in addition to the available information).
Yeah, we do change the values of module parameters. But a lot of them
are uid 0 only and we have PARAMS for those.
Do anything userspace use those actually? Do we plan to use them instead
of the getparams since now on?
> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre
--
Cheers,
Arek
More information about the Intel-gfx
mailing list