[Intel-gfx] [PATCH 8/8] drm/i915/get_params: Add HuC status to getparams

[Srivatsa, Anusha]

>-----Original Message-----
>From: Hiler, Arkadiusz
>Sent: Tuesday, December 13, 2016 1:41 AM
>To: Chris Wilson <chris at chris-wilson.co.uk>; Srivatsa, Anusha
><anusha.srivatsa at intel.com>; intel-gfx at lists.freedesktop.org; Peter Antoine
><peter.antoine at intel.com>
>Subject: Re: [Intel-gfx] [PATCH 8/8] drm/i915/get_params: Add HuC status to
>getparams
>
>On Thu, Dec 08, 2016 at 11:55:34PM +0000, Chris Wilson 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:
>> > +		value = intel_is_huc_valid(dev_priv);
>> > +		break;
>>
>> Why did you put it here? It breaks the pattern of case statements.
>>
>> >  	default:
>> >  		DRM_DEBUG("Unknown parameter %d\n", param->param);
>> >  		return -EINVAL;
>> > diff --git a/drivers/gpu/drm/i915/intel_huc_loader.c
>> > b/drivers/gpu/drm/i915/intel_huc_loader.c
>> > index 96fc727..6704cc8 100644
>> > --- a/drivers/gpu/drm/i915/intel_huc_loader.c
>> > +++ b/drivers/gpu/drm/i915/intel_huc_loader.c
>> > @@ -289,3 +289,15 @@ void intel_huc_fini(struct drm_device *dev)
>> >  	huc_fw->fetch_status = UC_FIRMWARE_NONE;  }
>> >
>> > +/**
>> > + * intel_is_huc_valid() - Check to see if the HuC is fully loaded.
>> > + * @dev_priv:	drm device to check.
>> > + *
>> > + * This function will return true if the guc has been loaded and
>> > + * has valid firmware. The simplest way of doing this is to check
>> > + * if the HuC has been validated, if so it must have been loaded.
>> > + */
>> > +int intel_is_huc_valid(struct drm_i915_private *dev_priv)
>>
>> bool
>>
>> > +{
>> > +	return ((I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED) != 0);
>>
>> (brackets (because (brackets)))
>>
>> But what I really wanted to ask... Does this register access require
>> the device to be awake and powered?
>
>Just confirmed that this register indeed requires MEDIA FW.

Thanks Arek. 

>> -Chris
>
>--
>Cheers,
>Arek