[Intel-gfx] [PATCH] drm/i915/huc: Check HuC status in dedicated function

[Michel Thierry]

On 14/03/18 13:04, Michal Wajdeczko wrote:
> We try to keep all HuC related code in dedicated file.
> There is no need to peek HuC register directly during
> handling getparam ioctl.
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
> Cc: Michel Thierry <michel.thierry at intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
> Cc: Anusha Srivatsa <anusha.srivatsa at intel.com>
> ---
>   drivers/gpu/drm/i915/i915_drv.c  |  6 +++---
>   drivers/gpu/drm/i915/intel_huc.c | 25 +++++++++++++++++++++++++
>   drivers/gpu/drm/i915/intel_huc.h |  1 +
>   3 files changed, 29 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index f03555e..a902e88 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -377,9 +377,9 @@ static int i915_getparam_ioctl(struct drm_device *dev, void *data,
>   		value = INTEL_INFO(dev_priv)->sseu.min_eu_in_pool;
>   		break;
>   	case I915_PARAM_HUC_STATUS:
> -		intel_runtime_pm_get(dev_priv);
> -		value = I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED;
> -		intel_runtime_pm_put(dev_priv);
> +		value = intel_huc_check_status(&dev_priv->huc);
> +		if (value < 0)
> +			return value;
>   		break;
>   	case I915_PARAM_MMAP_GTT_VERSION:
>   		/* Though we've started our numbering from 1, and so class all
> diff --git a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c
> index 1d6c47b..2912852 100644
> --- a/drivers/gpu/drm/i915/intel_huc.c
> +++ b/drivers/gpu/drm/i915/intel_huc.c
> @@ -92,3 +92,28 @@ int intel_huc_auth(struct intel_huc *huc)
>   	DRM_ERROR("HuC: Authentication failed %d\n", ret);
>   	return ret;
>   }
> +
> +/**
> + * intel_huc_check_status() - check HuC status
> + * @huc: intel_huc structure
> + *
> + * This function reads status register to verify if HuC
> + * firmware was successfully loaded.
> + *
> + * Returns positive value if HuC firmware is loaded and verified
> + * and -ENODEV if HuC is not present.

Before if huc was not loaded, get_param would just return 0 and the 
ioctl call would be OK. Maybe there's a test somewhere that would break? 
(I'm not arguing -ENODEV is better).

Otherwise,

Reviewed-by: Michel Thierry <michel.thierry at intel.com>

> + */
> +int intel_huc_check_status(struct intel_huc *huc)
> +{
> +	struct drm_i915_private *dev_priv = huc_to_i915(huc);
> +	u32 status;
> +
> +	if (!HAS_HUC(dev_priv))
> +		return -ENODEV;
> +
> +	intel_runtime_pm_get(dev_priv);
> +	status = I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED;
> +	intel_runtime_pm_put(dev_priv);
> +
> +	return status;
> +}
> diff --git a/drivers/gpu/drm/i915/intel_huc.h b/drivers/gpu/drm/i915/intel_huc.h
> index b185850..aa85490 100644
> --- a/drivers/gpu/drm/i915/intel_huc.h
> +++ b/drivers/gpu/drm/i915/intel_huc.h
> @@ -37,6 +37,7 @@ struct intel_huc {
>   
>   void intel_huc_init_early(struct intel_huc *huc);
>   int intel_huc_auth(struct intel_huc *huc);
> +int intel_huc_check_status(struct intel_huc *huc);
>   
>   static inline int intel_huc_sanitize(struct intel_huc *huc)
>   {
>