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@xxxxxxxxx> > > > > > > > > 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@xxxxxxxxx> > > > > Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@xxxxxxxxx> > > > > --- > > > > 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 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
