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

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

[Anusha Srivatsa]

From: Peter Antoine <peter.antoine@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.

Tested-by: Xiang Haihao <haihao.xiang@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
Signed-off-by: Peter Antoine <peter.antoine@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c         |  4 ++++
 drivers/gpu/drm/i915/intel_huc.h        |  2 +-
 drivers/gpu/drm/i915/intel_huc_loader.c | 14 ++++++++++++++
 include/uapi/drm/i915_drm.h             |  1 +
 4 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index ff1c18d..0d7b290 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_huc.h"
 #include "intel_guc.h"
 
 static struct drm_driver driver;
@@ -343,6 +344,9 @@ static int i915_getparam(struct drm_device *dev, void *data,
 	case I915_PARAM_HAS_GUC:
 		value = intel_is_guc_valid(dev_priv);
 		break;
+	case I915_PARAM_HAS_HUC:
+		value = intel_is_huc_valid(dev_priv);
+		break;
 	default:
 		DRM_DEBUG("Unknown parameter %d\n", param->param);
 		return -EINVAL;
diff --git a/drivers/gpu/drm/i915/intel_huc.h b/drivers/gpu/drm/i915/intel_huc.h
index 946caa7..5eac625 100644
--- a/drivers/gpu/drm/i915/intel_huc.h
+++ b/drivers/gpu/drm/i915/intel_huc.h
@@ -40,5 +40,5 @@ extern void intel_huc_init(struct drm_device *dev);
 extern int intel_huc_load(struct drm_device *dev);
 extern void intel_huc_auth(struct drm_device *dev);
 extern void intel_huc_fini(struct drm_device *dev);
-
+extern int intel_is_huc_valid(struct drm_i915_private *dev_priv);
 #endif
diff --git a/drivers/gpu/drm/i915/intel_huc_loader.c b/drivers/gpu/drm/i915/intel_huc_loader.c
index 87a6948..d574183 100644
--- a/drivers/gpu/drm/i915/intel_huc_loader.c
+++ b/drivers/gpu/drm/i915/intel_huc_loader.c
@@ -273,3 +273,17 @@ 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)
+{
+	return ((I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED) != 0);
+}
+
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 629fb5e..d236520 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -389,6 +389,7 @@ typedef struct drm_i915_irq_wait {
 #define I915_PARAM_MIN_EU_IN_POOL	 39
 #define I915_PARAM_MMAP_GTT_VERSION	 40
 #define I915_PARAM_HAS_GUC		 41
+#define I915_PARAM_HAS_HUC		 42
 
 typedef struct drm_i915_getparam {
 	__s32 param;
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Anusha Srivatsa]

From: Peter Antoine <peter.antoine@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.

Tested-by: Xiang Haihao <haihao.xiang@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
Signed-off-by: Peter Antoine <peter.antoine@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c         |  4 ++++
 drivers/gpu/drm/i915/intel_huc.h        |  2 +-
 drivers/gpu/drm/i915/intel_huc_loader.c | 14 ++++++++++++++
 include/uapi/drm/i915_drm.h             |  1 +
 4 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index ff1c18d..0d7b290 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_huc.h"
 #include "intel_guc.h"
 
 static struct drm_driver driver;
@@ -343,6 +344,9 @@ static int i915_getparam(struct drm_device *dev, void *data,
 	case I915_PARAM_HAS_GUC:
 		value = intel_is_guc_valid(dev_priv);
 		break;
+	case I915_PARAM_HAS_HUC:
+		value = intel_is_huc_valid(dev_priv);
+		break;
 	default:
 		DRM_DEBUG("Unknown parameter %d\n", param->param);
 		return -EINVAL;
diff --git a/drivers/gpu/drm/i915/intel_huc.h b/drivers/gpu/drm/i915/intel_huc.h
index 946caa7..5eac625 100644
--- a/drivers/gpu/drm/i915/intel_huc.h
+++ b/drivers/gpu/drm/i915/intel_huc.h
@@ -40,5 +40,5 @@ extern void intel_huc_init(struct drm_device *dev);
 extern int intel_huc_load(struct drm_device *dev);
 extern void intel_huc_auth(struct drm_device *dev);
 extern void intel_huc_fini(struct drm_device *dev);
-
+extern int intel_is_huc_valid(struct drm_i915_private *dev_priv);
 #endif
diff --git a/drivers/gpu/drm/i915/intel_huc_loader.c b/drivers/gpu/drm/i915/intel_huc_loader.c
index 87a6948..d574183 100644
--- a/drivers/gpu/drm/i915/intel_huc_loader.c
+++ b/drivers/gpu/drm/i915/intel_huc_loader.c
@@ -273,3 +273,17 @@ 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)
+{
+	return ((I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED) != 0);
+}
+
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 629fb5e..d236520 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -389,6 +389,7 @@ typedef struct drm_i915_irq_wait {
 #define I915_PARAM_MIN_EU_IN_POOL	 39
 #define I915_PARAM_MMAP_GTT_VERSION	 40
 #define I915_PARAM_HAS_GUC		 41
+#define I915_PARAM_HAS_HUC		 42
 
 typedef struct drm_i915_getparam {
 	__s32 param;
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 8/8] drm/i915/get_params: Add HuC status to getparams

[Rodrigo Vivi]

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

On Mon, Oct 03, 2016 at 11:43:02AM -0700, Anusha Srivatsa wrote:
> From: Peter Antoine <peter.antoine@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.
> 
> Tested-by: Xiang Haihao <haihao.xiang@intel.com>
> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> Signed-off-by: Peter Antoine <peter.antoine@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c         |  4 ++++
>  drivers/gpu/drm/i915/intel_huc.h        |  2 +-
>  drivers/gpu/drm/i915/intel_huc_loader.c | 14 ++++++++++++++
>  include/uapi/drm/i915_drm.h             |  1 +
>  4 files changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index ff1c18d..0d7b290 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_huc.h"
>  #include "intel_guc.h"
>  
>  static struct drm_driver driver;
> @@ -343,6 +344,9 @@ static int i915_getparam(struct drm_device *dev, void *data,
>  	case I915_PARAM_HAS_GUC:
>  		value = intel_is_guc_valid(dev_priv);
>  		break;
> +	case I915_PARAM_HAS_HUC:
> +		value = intel_is_huc_valid(dev_priv);
> +		break;
>  	default:
>  		DRM_DEBUG("Unknown parameter %d\n", param->param);
>  		return -EINVAL;
> diff --git a/drivers/gpu/drm/i915/intel_huc.h b/drivers/gpu/drm/i915/intel_huc.h
> index 946caa7..5eac625 100644
> --- a/drivers/gpu/drm/i915/intel_huc.h
> +++ b/drivers/gpu/drm/i915/intel_huc.h
> @@ -40,5 +40,5 @@ extern void intel_huc_init(struct drm_device *dev);
>  extern int intel_huc_load(struct drm_device *dev);
>  extern void intel_huc_auth(struct drm_device *dev);
>  extern void intel_huc_fini(struct drm_device *dev);
> -
> +extern int intel_is_huc_valid(struct drm_i915_private *dev_priv);
>  #endif
> diff --git a/drivers/gpu/drm/i915/intel_huc_loader.c b/drivers/gpu/drm/i915/intel_huc_loader.c
> index 87a6948..d574183 100644
> --- a/drivers/gpu/drm/i915/intel_huc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_huc_loader.c
> @@ -273,3 +273,17 @@ 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)
> +{
> +	return ((I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED) != 0);
> +}
> +
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index 629fb5e..d236520 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -389,6 +389,7 @@ typedef struct drm_i915_irq_wait {
>  #define I915_PARAM_MIN_EU_IN_POOL	 39
>  #define I915_PARAM_MMAP_GTT_VERSION	 40
>  #define I915_PARAM_HAS_GUC		 41
> +#define I915_PARAM_HAS_HUC		 42
>  
>  typedef struct drm_i915_getparam {
>  	__s32 param;
> -- 
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 8/8] drm/i915/get_params: Add HuC status to getparams

[Jeff McGee]

On Mon, Oct 03, 2016 at 11:43:02AM -0700, Anusha Srivatsa wrote:
> From: Peter Antoine <peter.antoine@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.
> 
> Tested-by: Xiang Haihao <haihao.xiang@intel.com>
> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> Signed-off-by: Peter Antoine <peter.antoine@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c         |  4 ++++
>  drivers/gpu/drm/i915/intel_huc.h        |  2 +-
>  drivers/gpu/drm/i915/intel_huc_loader.c | 14 ++++++++++++++
>  include/uapi/drm/i915_drm.h             |  1 +
>  4 files changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index ff1c18d..0d7b290 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_huc.h"
>  #include "intel_guc.h"
>  
>  static struct drm_driver driver;
> @@ -343,6 +344,9 @@ static int i915_getparam(struct drm_device *dev, void *data,
>  	case I915_PARAM_HAS_GUC:
>  		value = intel_is_guc_valid(dev_priv);
>  		break;
> +	case I915_PARAM_HAS_HUC:
> +		value = intel_is_huc_valid(dev_priv);
> +		break;
>  	default:
>  		DRM_DEBUG("Unknown parameter %d\n", param->param);
>  		return -EINVAL;
> diff --git a/drivers/gpu/drm/i915/intel_huc.h b/drivers/gpu/drm/i915/intel_huc.h
> index 946caa7..5eac625 100644
> --- a/drivers/gpu/drm/i915/intel_huc.h
> +++ b/drivers/gpu/drm/i915/intel_huc.h
> @@ -40,5 +40,5 @@ extern void intel_huc_init(struct drm_device *dev);
>  extern int intel_huc_load(struct drm_device *dev);
>  extern void intel_huc_auth(struct drm_device *dev);
>  extern void intel_huc_fini(struct drm_device *dev);
> -
> +extern int intel_is_huc_valid(struct drm_i915_private *dev_priv);
>  #endif
> diff --git a/drivers/gpu/drm/i915/intel_huc_loader.c b/drivers/gpu/drm/i915/intel_huc_loader.c
> index 87a6948..d574183 100644
> --- a/drivers/gpu/drm/i915/intel_huc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_huc_loader.c
> @@ -273,3 +273,17 @@ 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)
I'm still unclear on the 'intel' vs. 'i915' prefix usage. But it seems
that intel prefixed functions accept drm_device instead of drm_i915_private.

Also, need to see the userspace usage of this new GETPARAM.

> +{
> +	return ((I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED) != 0);
> +}
> +
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index 629fb5e..d236520 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -389,6 +389,7 @@ typedef struct drm_i915_irq_wait {
>  #define I915_PARAM_MIN_EU_IN_POOL	 39
>  #define I915_PARAM_MMAP_GTT_VERSION	 40
>  #define I915_PARAM_HAS_GUC		 41
> +#define I915_PARAM_HAS_HUC		 42
>  
>  typedef struct drm_i915_getparam {
>  	__s32 param;
> -- 
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 8/8] drm/i915/get_params: Add HuC status to getparams

[Daniel Vetter]

On Thu, Oct 13, 2016 at 02:47:32PM -0700, Jeff McGee wrote:
> On Mon, Oct 03, 2016 at 11:43:02AM -0700, Anusha Srivatsa wrote:
> > From: Peter Antoine <peter.antoine@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.
> > 
> > Tested-by: Xiang Haihao <haihao.xiang@intel.com>
> > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> > Signed-off-by: Peter Antoine <peter.antoine@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.c         |  4 ++++
> >  drivers/gpu/drm/i915/intel_huc.h        |  2 +-
> >  drivers/gpu/drm/i915/intel_huc_loader.c | 14 ++++++++++++++
> >  include/uapi/drm/i915_drm.h             |  1 +
> >  4 files changed, 20 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > index ff1c18d..0d7b290 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_huc.h"
> >  #include "intel_guc.h"
> >  
> >  static struct drm_driver driver;
> > @@ -343,6 +344,9 @@ static int i915_getparam(struct drm_device *dev, void *data,
> >  	case I915_PARAM_HAS_GUC:
> >  		value = intel_is_guc_valid(dev_priv);
> >  		break;
> > +	case I915_PARAM_HAS_HUC:
> > +		value = intel_is_huc_valid(dev_priv);
> > +		break;
> >  	default:
> >  		DRM_DEBUG("Unknown parameter %d\n", param->param);
> >  		return -EINVAL;
> > diff --git a/drivers/gpu/drm/i915/intel_huc.h b/drivers/gpu/drm/i915/intel_huc.h
> > index 946caa7..5eac625 100644
> > --- a/drivers/gpu/drm/i915/intel_huc.h
> > +++ b/drivers/gpu/drm/i915/intel_huc.h
> > @@ -40,5 +40,5 @@ extern void intel_huc_init(struct drm_device *dev);
> >  extern int intel_huc_load(struct drm_device *dev);
> >  extern void intel_huc_auth(struct drm_device *dev);
> >  extern void intel_huc_fini(struct drm_device *dev);
> > -
> > +extern int intel_is_huc_valid(struct drm_i915_private *dev_priv);
> >  #endif
> > diff --git a/drivers/gpu/drm/i915/intel_huc_loader.c b/drivers/gpu/drm/i915/intel_huc_loader.c
> > index 87a6948..d574183 100644
> > --- a/drivers/gpu/drm/i915/intel_huc_loader.c
> > +++ b/drivers/gpu/drm/i915/intel_huc_loader.c
> > @@ -273,3 +273,17 @@ 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)
> I'm still unclear on the 'intel' vs. 'i915' prefix usage. But it seems
> that intel prefixed functions accept drm_device instead of drm_i915_private.

intel_ is core and modeset, i915_ is gem stuff. mostly. Absolutely nothing
to do with drm_device vs. drm_i915_private, that should only be a question
of whether it's a drm vfunc or an internal one (we're transitioning to
that pattern, so atm it's a bit a mess).

> Also, need to see the userspace usage of this new GETPARAM.

+1 ;-)

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Anusha Srivatsa]

From: Peter Antoine <peter.antoine@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.
v6: rebased.
v7: rebased.

Tested-by: Xiang Haihao <haihao.xiang@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
Signed-off-by: Peter Antoine <peter.antoine@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Reviewed-by: Jeff McGee <jeff.mcgee@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c         |  5 +++++
 drivers/gpu/drm/i915/intel_huc.h        |  1 +
 drivers/gpu/drm/i915/intel_huc_loader.c | 14 ++++++++++++++
 include/uapi/drm/i915_drm.h             |  1 +
 4 files changed, 21 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index aa44d8d..52a1941 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -49,6 +49,8 @@
 #include "i915_trace.h"
 #include "i915_vgpu.h"
 #include "intel_drv.h"
+#include "intel_guc.h"
+#include "intel_huc.h"
 
 static struct drm_driver driver;
 
@@ -346,6 +348,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;
 	default:
 		DRM_DEBUG("Unknown parameter %d\n", param->param);
 		return -EINVAL;
diff --git a/drivers/gpu/drm/i915/intel_huc.h b/drivers/gpu/drm/i915/intel_huc.h
index 3ce0299..2e150be 100644
--- a/drivers/gpu/drm/i915/intel_huc.h
+++ b/drivers/gpu/drm/i915/intel_huc.h
@@ -39,4 +39,5 @@ struct intel_huc {
 void intel_huc_init(struct drm_device *dev);
 void intel_huc_fini(struct drm_device *dev);
 int intel_huc_load(struct drm_device *dev);
+extern int intel_is_huc_valid(struct drm_i915_private *dev_priv);
 #endif
diff --git a/drivers/gpu/drm/i915/intel_huc_loader.c b/drivers/gpu/drm/i915/intel_huc_loader.c
index a388437..df149b7 100644
--- a/drivers/gpu/drm/i915/intel_huc_loader.c
+++ b/drivers/gpu/drm/i915/intel_huc_loader.c
@@ -290,3 +290,17 @@ 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)
+{
+	return ((I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED) != 0);
+}
+
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 03725fe..aa7667e 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -388,6 +388,7 @@ typedef struct drm_i915_irq_wait {
 #define I915_PARAM_HAS_POOLED_EU	 38
 #define I915_PARAM_MIN_EU_IN_POOL	 39
 #define I915_PARAM_MMAP_GTT_VERSION	 40
+#define I915_PARAM_HAS_HUC		 42
 
 typedef struct drm_i915_getparam {
 	__s32 param;
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Anusha Srivatsa]

From: Peter Antoine <peter.antoine@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.
v6: rebased.
v7: rebased.

Tested-by: Xiang Haihao <haihao.xiang@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
Signed-off-by: Peter Antoine <peter.antoine@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Reviewed-by: Jeff McGee <jeff.mcgee@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c         |  8 ++++++++
 drivers/gpu/drm/i915/intel_huc.h        |  1 +
 drivers/gpu/drm/i915/intel_huc_loader.c | 14 ++++++++++++++
 include/uapi/drm/i915_drm.h             |  1 +
 4 files changed, 24 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 0213a30..52a1941 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -49,6 +49,8 @@
 #include "i915_trace.h"
 #include "i915_vgpu.h"
 #include "intel_drv.h"
+#include "intel_guc.h"
+#include "intel_huc.h"
 
 static struct drm_driver driver;
 
@@ -346,6 +348,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;
 	default:
 		DRM_DEBUG("Unknown parameter %d\n", param->param);
 		return -EINVAL;
@@ -599,6 +604,7 @@ static int i915_load_modeset_init(struct drm_device *dev)
 	if (ret)
 		goto cleanup_irq;
 
+	intel_huc_init(dev);
 	intel_guc_init(dev);
 
 	ret = i915_gem_init(dev);
@@ -626,6 +632,7 @@ static int i915_load_modeset_init(struct drm_device *dev)
 		DRM_ERROR("failed to idle hardware; continuing to unload!\n");
 	i915_gem_fini(dev_priv);
 cleanup_irq:
+	intel_huc_fini(dev);
 	intel_guc_fini(dev);
 	drm_irq_uninstall(dev);
 	intel_teardown_gmbus(dev);
@@ -1313,6 +1320,7 @@ void i915_driver_unload(struct drm_device *dev)
 	/* Flush any outstanding unpin_work. */
 	drain_workqueue(dev_priv->wq);
 
+	intel_huc_fini(dev);
 	intel_guc_fini(dev);
 	i915_gem_fini(dev_priv);
 	intel_fbc_cleanup_cfb(dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_huc.h b/drivers/gpu/drm/i915/intel_huc.h
index 3ce0299..2e150be 100644
--- a/drivers/gpu/drm/i915/intel_huc.h
+++ b/drivers/gpu/drm/i915/intel_huc.h
@@ -39,4 +39,5 @@ struct intel_huc {
 void intel_huc_init(struct drm_device *dev);
 void intel_huc_fini(struct drm_device *dev);
 int intel_huc_load(struct drm_device *dev);
+extern int intel_is_huc_valid(struct drm_i915_private *dev_priv);
 #endif
diff --git a/drivers/gpu/drm/i915/intel_huc_loader.c b/drivers/gpu/drm/i915/intel_huc_loader.c
index dab64ba..c6eec5b 100644
--- a/drivers/gpu/drm/i915/intel_huc_loader.c
+++ b/drivers/gpu/drm/i915/intel_huc_loader.c
@@ -292,3 +292,17 @@ 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)
+{
+	return ((I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED) != 0);
+}
+
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 03725fe..aa7667e 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -388,6 +388,7 @@ typedef struct drm_i915_irq_wait {
 #define I915_PARAM_HAS_POOLED_EU	 38
 #define I915_PARAM_MIN_EU_IN_POOL	 39
 #define I915_PARAM_MMAP_GTT_VERSION	 40
+#define I915_PARAM_HAS_HUC		 42
 
 typedef struct drm_i915_getparam {
 	__s32 param;
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Anusha Srivatsa]

From: Peter Antoine <peter.antoine@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.
v6: rebased.
v7: rebased.
v8: rebased.

Tested-by: Xiang Haihao <haihao.xiang@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
Signed-off-by: Peter Antoine <peter.antoine@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Reviewed-by: Jeff McGee <jeff.mcgee@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c         |  8 ++++++++
 drivers/gpu/drm/i915/intel_huc.h        |  1 +
 drivers/gpu/drm/i915/intel_huc_loader.c | 13 +++++++++++++
 include/uapi/drm/i915_drm.h             |  1 +
 4 files changed, 23 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index b893e67..f2d5b0f 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -49,6 +49,8 @@
 #include "i915_trace.h"
 #include "i915_vgpu.h"
 #include "intel_drv.h"
+#include "intel_guc.h"
+#include "intel_huc.h"
 
 static struct drm_driver driver;
 
@@ -350,6 +352,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;
 	default:
 		DRM_DEBUG("Unknown parameter %d\n", param->param);
 		return -EINVAL;
@@ -603,6 +608,7 @@ static int i915_load_modeset_init(struct drm_device *dev)
 	if (ret)
 		goto cleanup_irq;
 
+	intel_huc_init(dev);
 	intel_guc_init(dev);
 
 	ret = i915_gem_init(dev);
@@ -630,6 +636,7 @@ static int i915_load_modeset_init(struct drm_device *dev)
 		DRM_ERROR("failed to idle hardware; continuing to unload!\n");
 	i915_gem_fini(dev_priv);
 cleanup_irq:
+	intel_huc_fini(dev);
 	intel_guc_fini(dev);
 	drm_irq_uninstall(dev);
 	intel_teardown_gmbus(dev);
@@ -1325,6 +1332,7 @@ void i915_driver_unload(struct drm_device *dev)
 	/* Flush any outstanding unpin_work. */
 	drain_workqueue(dev_priv->wq);
 
+	intel_huc_fini(dev);
 	intel_guc_fini(dev);
 	i915_gem_fini(dev_priv);
 	intel_fbc_cleanup_cfb(dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_huc.h b/drivers/gpu/drm/i915/intel_huc.h
index 3ce0299..2e150be 100644
--- a/drivers/gpu/drm/i915/intel_huc.h
+++ b/drivers/gpu/drm/i915/intel_huc.h
@@ -39,4 +39,5 @@ struct intel_huc {
 void intel_huc_init(struct drm_device *dev);
 void intel_huc_fini(struct drm_device *dev);
 int intel_huc_load(struct drm_device *dev);
+extern int intel_is_huc_valid(struct drm_i915_private *dev_priv);
 #endif
diff --git a/drivers/gpu/drm/i915/intel_huc_loader.c b/drivers/gpu/drm/i915/intel_huc_loader.c
index 3a2555e..8c3993b 100644
--- a/drivers/gpu/drm/i915/intel_huc_loader.c
+++ b/drivers/gpu/drm/i915/intel_huc_loader.c
@@ -291,3 +291,16 @@ 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)
+{
+	return ((I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED) != 0);
+}
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index bdfc688..0a9dac4 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -390,6 +390,7 @@ typedef struct drm_i915_irq_wait {
 #define I915_PARAM_HAS_POOLED_EU	 38
 #define I915_PARAM_MIN_EU_IN_POOL	 39
 #define I915_PARAM_MMAP_GTT_VERSION	 40
+#define I915_PARAM_HAS_HUC		 42
 
 /* Query whether DRM_I915_GEM_EXECBUFFER2 supports user defined execution
  * priorities and the driver will attempt to execute batches in priority order.
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 8/8] drm/i915/get_params: Add HuC status to getparams

[Jeff McGee]

On Wed, Nov 23, 2016 at 02:27:43PM -0800, Anusha Srivatsa wrote:
> From: Peter Antoine <peter.antoine@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.
> v6: rebased.
> v7: rebased.
> v8: rebased.
> 
> Tested-by: Xiang Haihao <haihao.xiang@intel.com>
> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> Signed-off-by: Peter Antoine <peter.antoine@intel.com>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Reviewed-by: Jeff McGee <jeff.mcgee@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c         |  8 ++++++++
>  drivers/gpu/drm/i915/intel_huc.h        |  1 +
>  drivers/gpu/drm/i915/intel_huc_loader.c | 13 +++++++++++++
>  include/uapi/drm/i915_drm.h             |  1 +
>  4 files changed, 23 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index b893e67..f2d5b0f 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -49,6 +49,8 @@
>  #include "i915_trace.h"
>  #include "i915_vgpu.h"
>  #include "intel_drv.h"
> +#include "intel_guc.h"
> +#include "intel_huc.h"
>  
>  static struct drm_driver driver;
>  
> @@ -350,6 +352,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;
>  	default:
>  		DRM_DEBUG("Unknown parameter %d\n", param->param);
>  		return -EINVAL;
> @@ -603,6 +608,7 @@ static int i915_load_modeset_init(struct drm_device *dev)
>  	if (ret)
>  		goto cleanup_irq;
>  
> +	intel_huc_init(dev);
As we discussed, this needs to be in patch 3.

>  	intel_guc_init(dev);
>  
>  	ret = i915_gem_init(dev);
> @@ -630,6 +636,7 @@ static int i915_load_modeset_init(struct drm_device *dev)
>  		DRM_ERROR("failed to idle hardware; continuing to unload!\n");
>  	i915_gem_fini(dev_priv);
>  cleanup_irq:
> +	intel_huc_fini(dev);
>  	intel_guc_fini(dev);
>  	drm_irq_uninstall(dev);
>  	intel_teardown_gmbus(dev);
> @@ -1325,6 +1332,7 @@ void i915_driver_unload(struct drm_device *dev)
>  	/* Flush any outstanding unpin_work. */
>  	drain_workqueue(dev_priv->wq);
>  
> +	intel_huc_fini(dev);
>  	intel_guc_fini(dev);
>  	i915_gem_fini(dev_priv);
>  	intel_fbc_cleanup_cfb(dev_priv);
> diff --git a/drivers/gpu/drm/i915/intel_huc.h b/drivers/gpu/drm/i915/intel_huc.h
> index 3ce0299..2e150be 100644
> --- a/drivers/gpu/drm/i915/intel_huc.h
> +++ b/drivers/gpu/drm/i915/intel_huc.h
> @@ -39,4 +39,5 @@ struct intel_huc {
>  void intel_huc_init(struct drm_device *dev);
>  void intel_huc_fini(struct drm_device *dev);
>  int intel_huc_load(struct drm_device *dev);
> +extern int intel_is_huc_valid(struct drm_i915_private *dev_priv);
>  #endif
> diff --git a/drivers/gpu/drm/i915/intel_huc_loader.c b/drivers/gpu/drm/i915/intel_huc_loader.c
> index 3a2555e..8c3993b 100644
> --- a/drivers/gpu/drm/i915/intel_huc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_huc_loader.c
> @@ -291,3 +291,16 @@ 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)
> +{
> +	return ((I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED) != 0);
> +}
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index bdfc688..0a9dac4 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -390,6 +390,7 @@ typedef struct drm_i915_irq_wait {
>  #define I915_PARAM_HAS_POOLED_EU	 38
>  #define I915_PARAM_MIN_EU_IN_POOL	 39
>  #define I915_PARAM_MMAP_GTT_VERSION	 40
> +#define I915_PARAM_HAS_HUC		 42
>  
>  /* Query whether DRM_I915_GEM_EXECBUFFER2 supports user defined execution
>   * priorities and the driver will attempt to execute batches in priority order.
> -- 
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Anusha Srivatsa]

From: Peter Antoine <peter.antoine@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.

Signed-off-by: Peter Antoine <peter.antoine@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c         |  5 +++++
 drivers/gpu/drm/i915/intel_huc.h        |  1 +
 drivers/gpu/drm/i915/intel_huc_loader.c | 12 ++++++++++++
 include/uapi/drm/i915_drm.h             |  1 +
 4 files changed, 19 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 075d9ce..75a3e24 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -49,6 +49,8 @@
 #include "i915_trace.h"
 #include "i915_vgpu.h"
 #include "intel_drv.h"
+#include "intel_uc.h"
+#include "intel_huc.h"
 
 static struct drm_driver driver;
 
@@ -350,6 +352,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;
 	default:
 		DRM_DEBUG("Unknown parameter %d\n", param->param);
 		return -EINVAL;
diff --git a/drivers/gpu/drm/i915/intel_huc.h b/drivers/gpu/drm/i915/intel_huc.h
index 1dd18c5..1b67311 100644
--- a/drivers/gpu/drm/i915/intel_huc.h
+++ b/drivers/gpu/drm/i915/intel_huc.h
@@ -39,4 +39,5 @@ struct intel_huc {
 void intel_huc_init(struct drm_device *dev);
 void intel_huc_fini(struct drm_device *dev);
 int intel_huc_load(struct drm_device *dev);
+int intel_is_huc_valid(struct drm_i915_private *dev_priv);
 #endif
diff --git a/drivers/gpu/drm/i915/intel_huc_loader.c b/drivers/gpu/drm/i915/intel_huc_loader.c
index 20526a4..e18de0f6 100644
--- a/drivers/gpu/drm/i915/intel_huc_loader.c
+++ b/drivers/gpu/drm/i915/intel_huc_loader.c
@@ -292,3 +292,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)
+{
+	return ((I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED) != 0);
+}
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index bdfc688..397b47d 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -395,6 +395,7 @@ typedef struct drm_i915_irq_wait {
  * priorities and the driver will attempt to execute batches in priority order.
  */
 #define I915_PARAM_HAS_SCHEDULER	 41
+#define I915_PARAM_HAS_HUC		 42
 
 typedef struct drm_i915_getparam {
 	__s32 param;
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 8/8] drm/i915/get_params: Add HuC status to getparams

[Arkadiusz Hiler]

On Wed, Nov 30, 2016 at 03:31:34PM -0800, Anusha Srivatsa wrote:
> From: Peter Antoine <peter.antoine@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.
> 
> Signed-off-by: Peter Antoine <peter.antoine@intel.com>
Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c         |  5 +++++
>  drivers/gpu/drm/i915/intel_huc.h        |  1 +
>  drivers/gpu/drm/i915/intel_huc_loader.c | 12 ++++++++++++
>  include/uapi/drm/i915_drm.h             |  1 +
>  4 files changed, 19 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 075d9ce..75a3e24 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -49,6 +49,8 @@
>  #include "i915_trace.h"
>  #include "i915_vgpu.h"
>  #include "intel_drv.h"
> +#include "intel_uc.h"
> +#include "intel_huc.h"
>  
>  static struct drm_driver driver;
>  
> @@ -350,6 +352,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;
>  	default:
>  		DRM_DEBUG("Unknown parameter %d\n", param->param);
>  		return -EINVAL;
> diff --git a/drivers/gpu/drm/i915/intel_huc.h b/drivers/gpu/drm/i915/intel_huc.h
> index 1dd18c5..1b67311 100644
> --- a/drivers/gpu/drm/i915/intel_huc.h
> +++ b/drivers/gpu/drm/i915/intel_huc.h
> @@ -39,4 +39,5 @@ struct intel_huc {
>  void intel_huc_init(struct drm_device *dev);
>  void intel_huc_fini(struct drm_device *dev);
>  int intel_huc_load(struct drm_device *dev);
> +int intel_is_huc_valid(struct drm_i915_private *dev_priv);
>  #endif
> diff --git a/drivers/gpu/drm/i915/intel_huc_loader.c b/drivers/gpu/drm/i915/intel_huc_loader.c
> index 20526a4..e18de0f6 100644
> --- a/drivers/gpu/drm/i915/intel_huc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_huc_loader.c
> @@ -292,3 +292,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)
> +{
> +	return ((I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED) != 0);
> +}
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index bdfc688..397b47d 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -395,6 +395,7 @@ typedef struct drm_i915_irq_wait {
>   * priorities and the driver will attempt to execute batches in priority order.
>   */
>  #define I915_PARAM_HAS_SCHEDULER	 41
> +#define I915_PARAM_HAS_HUC		 42
>  
>  typedef struct drm_i915_getparam {
>  	__s32 param;
> -- 
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Cheers,
Arek
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[anushasr]

From: Peter Antoine <peter.antoine@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@intel.com>
Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@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;
 	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)
+{
+	return ((I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED) != 0);
+}
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 1db8bc2..ccd3f69 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -226,6 +226,7 @@ int i915_guc_log_control(struct drm_i915_private *dev_priv, u64 control_val);
 void intel_huc_init(struct drm_i915_private *dev_priv);
 void intel_huc_fini(struct drm_device *dev);
 int intel_huc_load(struct drm_i915_private *dev_priv);
+int intel_is_huc_valid(struct drm_i915_private *dev_priv);
 
 #endif
 #endif
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index da32c2f..3e1964c 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -395,6 +395,7 @@ typedef struct drm_i915_irq_wait {
  * priorities and the driver will attempt to execute batches in priority order.
  */
 #define I915_PARAM_HAS_SCHEDULER	 41
+#define I915_PARAM_HAS_HUC		 42
 
 typedef struct drm_i915_getparam {
 	__s32 param;
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 8/8] drm/i915/get_params: Add HuC status to getparams

[Chris Wilson]

On Thu, Dec 08, 2016 at 03:02:19PM -0800, anushasr wrote:
> From: Peter Antoine <peter.antoine@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@intel.com>
> Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@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?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 8/8] drm/i915/get_params: Add HuC status to getparams

[Michal Wajdeczko]

On Thu, Dec 08, 2016 at 03:02:19PM -0800, anushasr wrote:
> From: Peter Antoine <peter.antoine@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@intel.com>
> Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@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)


> +		value = intel_is_huc_valid(dev_priv);
> +		break;
>  	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

Please change function return type to bool to match this description.


> + * 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)
> +{
> +	return ((I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED) != 0);
> +}
> diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
> index 1db8bc2..ccd3f69 100644
> --- a/drivers/gpu/drm/i915/intel_uc.h
> +++ b/drivers/gpu/drm/i915/intel_uc.h
> @@ -226,6 +226,7 @@ int i915_guc_log_control(struct drm_i915_private *dev_priv, u64 control_val);
>  void intel_huc_init(struct drm_i915_private *dev_priv);
>  void intel_huc_fini(struct drm_device *dev);
>  int intel_huc_load(struct drm_i915_private *dev_priv);
> +int intel_is_huc_valid(struct drm_i915_private *dev_priv);
>  
>  #endif
>  #endif
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index da32c2f..3e1964c 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -395,6 +395,7 @@ typedef struct drm_i915_irq_wait {
>   * priorities and the driver will attempt to execute batches in priority order.
>   */
>  #define I915_PARAM_HAS_SCHEDULER	 41
> +#define I915_PARAM_HAS_HUC		 42
>  
>  typedef struct drm_i915_getparam {
>  	__s32 param;
> -- 
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 8/8] drm/i915/get_params: Add HuC status to getparams

[Arkadiusz Hiler]

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@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@intel.com>
> > Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@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.

> > +		value = intel_is_huc_valid(dev_priv);
> > +		break;
> >  	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
> 
> Please change function return type to bool to match this description.
> 
> > + * 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)
> > +{
> > +	return ((I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED) != 0);
> > +}
> > diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
> > index 1db8bc2..ccd3f69 100644
> > --- a/drivers/gpu/drm/i915/intel_uc.h
> > +++ b/drivers/gpu/drm/i915/intel_uc.h
> > @@ -226,6 +226,7 @@ int i915_guc_log_control(struct drm_i915_private *dev_priv, u64 control_val);
> >  void intel_huc_init(struct drm_i915_private *dev_priv);
> >  void intel_huc_fini(struct drm_device *dev);
> >  int intel_huc_load(struct drm_i915_private *dev_priv);
> > +int intel_is_huc_valid(struct drm_i915_private *dev_priv);
> >  
> >  #endif
> >  #endif
> > diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> > index da32c2f..3e1964c 100644
> > --- a/include/uapi/drm/i915_drm.h
> > +++ b/include/uapi/drm/i915_drm.h
> > @@ -395,6 +395,7 @@ typedef struct drm_i915_irq_wait {
> >   * priorities and the driver will attempt to execute batches in priority order.
> >   */
> >  #define I915_PARAM_HAS_SCHEDULER	 41
> > +#define I915_PARAM_HAS_HUC		 42
> >  
> >  typedef struct drm_i915_getparam {
> >  	__s32 param;
> > -- 
> > 2.7.4
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Cheers,
Arek
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 8/8] drm/i915/get_params: Add HuC status to getparams

[Chris Wilson]

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@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@intel.com>
> > > Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@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?

Are there any other users of intel_huc_is_valid()?

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).
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 8/8] drm/i915/get_params: Add HuC status to getparams

[Arkadiusz Hiler]

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

Re: [PATCH 8/8] drm/i915/get_params: Add HuC status to getparams

[Chris Wilson]

On Mon, Dec 12, 2016 at 03:52:05PM +0100, Arkadiusz Hiler wrote:
> On Mon, Dec 12, 2016 at 02:21:41PM +0000, Chris Wilson wrote:
> > 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?

I've done both from userspace... I don't really have a preference, once
you have an fd, an ioctl/GETPARAM is trivial. But for quick querying and
reporting of driver state, reading the module parameter is easier. So I
have a tendency to use an ioctl in production code and module parameter
in igt. (And I would say that for one-off validation purposes, i.e. did
hte module actually enable huc?, just check the module parameter. If
userspace is expected to interact and respond to the setting during its
early probe, make it an ioctl so it fits in with the similar code also
being run at that time.)

It is just worth explaining the intended usecase in the cover note, so
that the use can be sanity checked and reflected upon later if need be.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 8/8] drm/i915/get_params: Add HuC status to getparams

[Arkadiusz Hiler]

On Mon, Dec 12, 2016 at 03:17:16PM +0000, Chris Wilson wrote:
> On Mon, Dec 12, 2016 at 03:52:05PM +0100, Arkadiusz Hiler wrote:
> > On Mon, Dec 12, 2016 at 02:21:41PM +0000, Chris Wilson wrote:
> > > 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?
> 
> I've done both from userspace... I don't really have a preference, once
> you have an fd, an ioctl/GETPARAM is trivial. But for quick querying and
> reporting of driver state, reading the module parameter is easier. So I
> have a tendency to use an ioctl in production code and module parameter
> in igt. (And I would say that for one-off validation purposes, i.e. did
> hte module actually enable huc?, just check the module parameter.

Thank you for explaining it :)

> If  userspace is expected to interact and respond to the setting
> during its early probe, make it an ioctl so it fits in with the
> similar code also being run at that time.)

Okay. Seems like the scenario Anusha's got.

> It is just worth explaining the intended usecase in the cover note, so
> that the use can be sanity checked and reflected upon later if need be.
> -Chris

The cover letter links patches from the libva which is pretty much a
template usecase, but I agree, it wold be nice if it used
words/pseudocode instead of portions of code from some other project.

> -- 
> Chris Wilson, Intel Open Source Technology Centre

-- 
Cheers,
Arek
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 8/8] drm/i915/get_params: Add HuC status to getparams

[Srivatsa, Anusha]

>-----Original Message-----
>From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
>Chris Wilson
>Sent: Monday, December 12, 2016 7:17 AM
>To: Hiler, Arkadiusz <arkadiusz.hiler@intel.com>
>Cc: intel-gfx@lists.freedesktop.org; Michal Wajdeczko
><michal.wajdeczko@linux.intel.com>
>Subject: Re: [Intel-gfx] [PATCH 8/8] drm/i915/get_params: Add HuC status to
>getparams
>
>On Mon, Dec 12, 2016 at 03:52:05PM +0100, Arkadiusz Hiler wrote:
>> On Mon, Dec 12, 2016 at 02:21:41PM +0000, Chris Wilson wrote:
>> > 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?
>
>I've done both from userspace... I don't really have a preference, once you have
>an fd, an ioctl/GETPARAM is trivial. But for quick querying and reporting of driver
>state, reading the module parameter is easier. So I have a tendency to use an
>ioctl in production code and module parameter in igt. (And I would say that for
>one-off validation purposes, i.e. did hte module actually enable huc?, just check
>the module parameter. If userspace is expected to interact and respond to the
>setting during its early probe, make it an ioctl so it fits in with the similar code
>also being run at that time.)
>
>It is just worth explaining the intended usecase in the cover note, so that the use
>can be sanity checked and reflected upon later if need be.
>-Chris
>
Thanks for explaining Chris....

Anusha
>Chris Wilson, Intel Open Source Technology Centre
>_______________________________________________
>Intel-gfx mailing list
>Intel-gfx@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 8/8] drm/i915/get_params: Add HuC status to getparams

[Arkadiusz Hiler]

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@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@intel.com>
> > Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@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.

> -Chris

-- 
Cheers,
Arek
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [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@chris-wilson.co.uk>; Srivatsa, Anusha
><anusha.srivatsa@intel.com>; intel-gfx@lists.freedesktop.org; Peter Antoine
><peter.antoine@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@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@intel.com>
>> > Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[anushasr]

From: Peter Antoine <peter.antoine@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
v7: rebased. Rename I915_PARAM_HAS_HUC to I915_PARAM_HUC_STATUS.
Remove intel_is_huc_valid() since it is used only in one place.
Put the case of I915_PARAM_HAS_HUC() in the right place.

Signed-off-by: Peter Antoine <peter.antoine@intel.com>
Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c         | 4 ++++
 drivers/gpu/drm/i915/intel_huc_loader.c | 1 -
 include/uapi/drm/i915_drm.h             | 1 +
 3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 85a47c2..0bc016d 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;
 
@@ -315,6 +316,9 @@ static int i915_getparam(struct drm_device *dev, void *data,
 	case I915_PARAM_MIN_EU_IN_POOL:
 		value = INTEL_INFO(dev_priv)->sseu.min_eu_in_pool;
 		break;
+	case I915_PARAM_HUC_STATUS:
+		value = I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED;
+		break;
 	case I915_PARAM_MMAP_GTT_VERSION:
 		/* Though we've started our numbering from 1, and so class all
 		 * earlier versions as 0, in effect their value is undefined as
diff --git a/drivers/gpu/drm/i915/intel_huc_loader.c b/drivers/gpu/drm/i915/intel_huc_loader.c
index d8c5266..b06a613 100644
--- a/drivers/gpu/drm/i915/intel_huc_loader.c
+++ b/drivers/gpu/drm/i915/intel_huc_loader.c
@@ -288,4 +288,3 @@ void intel_huc_fini(struct drm_device *dev)
 
 	huc_fw->fetch_status = INTEL_UC_FIRMWARE_NONE;
 }
-
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index da32c2f..57093b4 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -395,6 +395,7 @@ typedef struct drm_i915_irq_wait {
  * priorities and the driver will attempt to execute batches in priority order.
  */
 #define I915_PARAM_HAS_SCHEDULER	 41
+#define I915_PARAM_HUC_STATUS		 42
 
 typedef struct drm_i915_getparam {
 	__s32 param;
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 8/8] drm/i915/get_params: Add HuC status to getparams

[Chris Wilson]

On Thu, Dec 15, 2016 at 02:29:50PM -0800, anushasr wrote:
> From: Peter Antoine <peter.antoine@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
> v7: rebased. Rename I915_PARAM_HAS_HUC to I915_PARAM_HUC_STATUS.
> Remove intel_is_huc_valid() since it is used only in one place.
> Put the case of I915_PARAM_HAS_HUC() in the right place.
> 
> Signed-off-by: Peter Antoine <peter.antoine@intel.com>
> Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c         | 4 ++++
>  drivers/gpu/drm/i915/intel_huc_loader.c | 1 -
>  include/uapi/drm/i915_drm.h             | 1 +
>  3 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 85a47c2..0bc016d 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;
>  
> @@ -315,6 +316,9 @@ static int i915_getparam(struct drm_device *dev, void *data,
>  	case I915_PARAM_MIN_EU_IN_POOL:
>  		value = INTEL_INFO(dev_priv)->sseu.min_eu_in_pool;
>  		break;
> +	case I915_PARAM_HUC_STATUS:
> +		value = I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED;

Same question as last time: does the device need to be awake? We know is
one of the GT power wells, so presumably we need an rpm_get/rpm_put as
well to access the register.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 8/8] drm/i915/get_params: Add HuC status to getparams

[Arkadiusz Hiler]

On Thu, Dec 15, 2016 at 10:42:53PM +0000, Chris Wilson wrote:
> On Thu, Dec 15, 2016 at 02:29:50PM -0800, anushasr wrote:
> > From: Peter Antoine <peter.antoine@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
> > v7: rebased. Rename I915_PARAM_HAS_HUC to I915_PARAM_HUC_STATUS.
> > Remove intel_is_huc_valid() since it is used only in one place.
> > Put the case of I915_PARAM_HAS_HUC() in the right place.
> > 
> > Signed-off-by: Peter Antoine <peter.antoine@intel.com>
> > Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.c         | 4 ++++
> >  drivers/gpu/drm/i915/intel_huc_loader.c | 1 -
> >  include/uapi/drm/i915_drm.h             | 1 +
> >  3 files changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > index 85a47c2..0bc016d 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;
> >  
> > @@ -315,6 +316,9 @@ static int i915_getparam(struct drm_device *dev, void *data,
> >  	case I915_PARAM_MIN_EU_IN_POOL:
> >  		value = INTEL_INFO(dev_priv)->sseu.min_eu_in_pool;
> >  		break;
> > +	case I915_PARAM_HUC_STATUS:
> > +		value = I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED;
> 
> Same question as last time: does the device need to be awake? We know is
> one of the GT power wells, so presumably we need an rpm_get/rpm_put as
> well to access the register.
> -Chris

I get:

[ 1588.570174] [drm:i915_huc_load_status_info [i915]] HUC_STATUS2 PRE  24704
[ 1588.571285] [drm:intel_runtime_suspend [i915]] Suspending device
[ 1588.575768] [drm:intel_runtime_suspend [i915]] Device suspended
[ 1588.577156] [drm:i915_huc_load_status_info [i915]] HUC_STATUS2 POST 24704
[ 1588.578259] [drm:intel_runtime_resume [i915]] Resuming device

consistently from:

value = I915_READ(HUC_STATUS2);
DRM_DEBUG_DRIVER("HUC_STATUS2 PRE  %d\n", value);
i915_pm_ops.runtime_suspend(dev_priv->drm.dev);

value = I915_READ(HUC_STATUS2);
DRM_DEBUG_DRIVER("HUC_STATUS2 POST %d\n", value);
i915_pm_ops.runtime_resume(dev_priv->drm.dev);


> -- 
> Chris Wilson, Intel Open Source Technology Centre
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Cheers,
Arek
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 8/8] drm/i915/get_params: Add HuC status to getparams

[Arkadiusz Hiler]

On Thu, Dec 15, 2016 at 02:29:50PM -0800, anushasr wrote:
> From: Peter Antoine <peter.antoine@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
> v7: rebased. Rename I915_PARAM_HAS_HUC to I915_PARAM_HUC_STATUS.
> Remove intel_is_huc_valid() since it is used only in one place.
> Put the case of I915_PARAM_HAS_HUC() in the right place.
> 
> Signed-off-by: Peter Antoine <peter.antoine@intel.com>
> Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>

You've retained my rb without asking me.

With the changes you've made and confirmation that MEDIA FW that
I915_READ() assumes:

Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_drv.c         | 4 ++++
>  drivers/gpu/drm/i915/intel_huc_loader.c | 1 -
>  include/uapi/drm/i915_drm.h             | 1 +
>  3 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 85a47c2..0bc016d 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;
>  
> @@ -315,6 +316,9 @@ static int i915_getparam(struct drm_device *dev, void *data,
>  	case I915_PARAM_MIN_EU_IN_POOL:
>  		value = INTEL_INFO(dev_priv)->sseu.min_eu_in_pool;
>  		break;
> +	case I915_PARAM_HUC_STATUS:
> +		value = I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED;
> +		break;
>  	case I915_PARAM_MMAP_GTT_VERSION:
>  		/* Though we've started our numbering from 1, and so class all
>  		 * earlier versions as 0, in effect their value is undefined as
> diff --git a/drivers/gpu/drm/i915/intel_huc_loader.c b/drivers/gpu/drm/i915/intel_huc_loader.c
> index d8c5266..b06a613 100644
> --- a/drivers/gpu/drm/i915/intel_huc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_huc_loader.c
> @@ -288,4 +288,3 @@ void intel_huc_fini(struct drm_device *dev)
>  
>  	huc_fw->fetch_status = INTEL_UC_FIRMWARE_NONE;
>  }
> -
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index da32c2f..57093b4 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -395,6 +395,7 @@ typedef struct drm_i915_irq_wait {
>   * priorities and the driver will attempt to execute batches in priority order.
>   */
>  #define I915_PARAM_HAS_SCHEDULER	 41
> +#define I915_PARAM_HUC_STATUS		 42
>  
>  typedef struct drm_i915_getparam {
>  	__s32 param;
> -- 
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Cheers,
Arek
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 8/8] drm/i915/get_params: Add HuC status to getparams

[Chris Wilson]

On Fri, Dec 16, 2016 at 03:43:46PM +0100, Arkadiusz Hiler wrote:
> On Thu, Dec 15, 2016 at 10:42:53PM +0000, Chris Wilson wrote:
> > On Thu, Dec 15, 2016 at 02:29:50PM -0800, anushasr wrote:
> > > From: Peter Antoine <peter.antoine@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
> > > v7: rebased. Rename I915_PARAM_HAS_HUC to I915_PARAM_HUC_STATUS.
> > > Remove intel_is_huc_valid() since it is used only in one place.
> > > Put the case of I915_PARAM_HAS_HUC() in the right place.
> > > 
> > > Signed-off-by: Peter Antoine <peter.antoine@intel.com>
> > > Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_drv.c         | 4 ++++
> > >  drivers/gpu/drm/i915/intel_huc_loader.c | 1 -
> > >  include/uapi/drm/i915_drm.h             | 1 +
> > >  3 files changed, 5 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > > index 85a47c2..0bc016d 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;
> > >  
> > > @@ -315,6 +316,9 @@ static int i915_getparam(struct drm_device *dev, void *data,
> > >  	case I915_PARAM_MIN_EU_IN_POOL:
> > >  		value = INTEL_INFO(dev_priv)->sseu.min_eu_in_pool;
> > >  		break;
> > > +	case I915_PARAM_HUC_STATUS:
> > > +		value = I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED;
> > 
> > Same question as last time: does the device need to be awake? We know is
> > one of the GT power wells, so presumably we need an rpm_get/rpm_put as
> > well to access the register.
> > -Chris
> 
> I get:
> 
> [ 1588.570174] [drm:i915_huc_load_status_info [i915]] HUC_STATUS2 PRE  24704
> [ 1588.571285] [drm:intel_runtime_suspend [i915]] Suspending device
> [ 1588.575768] [drm:intel_runtime_suspend [i915]] Device suspended
> [ 1588.577156] [drm:i915_huc_load_status_info [i915]] HUC_STATUS2 POST 24704
> [ 1588.578259] [drm:intel_runtime_resume [i915]] Resuming device
> 
> consistently from:
> 
> value = I915_READ(HUC_STATUS2);
> DRM_DEBUG_DRIVER("HUC_STATUS2 PRE  %d\n", value);
> i915_pm_ops.runtime_suspend(dev_priv->drm.dev);
> 
> value = I915_READ(HUC_STATUS2);
> DRM_DEBUG_DRIVER("HUC_STATUS2 POST %d\n", value);
> i915_pm_ops.runtime_resume(dev_priv->drm.dev);

Also do the test with i915.mmio_debug=9999
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 8/8] drm/i915/get_params: Add HuC status to getparams

[Arkadiusz Hiler]

On Fri, Dec 16, 2016 at 04:12:36PM +0000, Chris Wilson wrote:
> On Fri, Dec 16, 2016 at 03:43:46PM +0100, Arkadiusz Hiler wrote:
> > On Thu, Dec 15, 2016 at 10:42:53PM +0000, Chris Wilson wrote:
> > > On Thu, Dec 15, 2016 at 02:29:50PM -0800, anushasr wrote:
> > > > From: Peter Antoine <peter.antoine@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
> > > > v7: rebased. Rename I915_PARAM_HAS_HUC to I915_PARAM_HUC_STATUS.
> > > > Remove intel_is_huc_valid() since it is used only in one place.
> > > > Put the case of I915_PARAM_HAS_HUC() in the right place.
> > > > 
> > > > Signed-off-by: Peter Antoine <peter.antoine@intel.com>
> > > > Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/i915_drv.c         | 4 ++++
> > > >  drivers/gpu/drm/i915/intel_huc_loader.c | 1 -
> > > >  include/uapi/drm/i915_drm.h             | 1 +
> > > >  3 files changed, 5 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > > > index 85a47c2..0bc016d 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;
> > > >  
> > > > @@ -315,6 +316,9 @@ static int i915_getparam(struct drm_device *dev, void *data,
> > > >  	case I915_PARAM_MIN_EU_IN_POOL:
> > > >  		value = INTEL_INFO(dev_priv)->sseu.min_eu_in_pool;
> > > >  		break;
> > > > +	case I915_PARAM_HUC_STATUS:
> > > > +		value = I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED;
> > > 
> > > Same question as last time: does the device need to be awake? We know is
> > > one of the GT power wells, so presumably we need an rpm_get/rpm_put as
> > > well to access the register.
> > > -Chris
> > 
> > I get:
> > 
> > [ 1588.570174] [drm:i915_huc_load_status_info [i915]] HUC_STATUS2 PRE  24704
> > [ 1588.571285] [drm:intel_runtime_suspend [i915]] Suspending device
> > [ 1588.575768] [drm:intel_runtime_suspend [i915]] Device suspended
> > [ 1588.577156] [drm:i915_huc_load_status_info [i915]] HUC_STATUS2 POST 24704
> > [ 1588.578259] [drm:intel_runtime_resume [i915]] Resuming device
> > 
> > consistently from:
> > 
> > value = I915_READ(HUC_STATUS2);
> > DRM_DEBUG_DRIVER("HUC_STATUS2 PRE  %d\n", value);
> > i915_pm_ops.runtime_suspend(dev_priv->drm.dev);
> > 
> > value = I915_READ(HUC_STATUS2);
> > DRM_DEBUG_DRIVER("HUC_STATUS2 POST %d\n", value);
> > i915_pm_ops.runtime_resume(dev_priv->drm.dev);
> 
> Also do the test with i915.mmio_debug=9999
> -Chris

Same effect. Works.

-- 
Cheers,
Arek
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 8/8] drm/i915/get_params: Add HuC status to getparams

[Chris Wilson]

On Fri, Dec 16, 2016 at 05:21:38PM +0100, Arkadiusz Hiler wrote:
> On Fri, Dec 16, 2016 at 04:12:36PM +0000, Chris Wilson wrote:
> > On Fri, Dec 16, 2016 at 03:43:46PM +0100, Arkadiusz Hiler wrote:
> > > On Thu, Dec 15, 2016 at 10:42:53PM +0000, Chris Wilson wrote:
> > > > On Thu, Dec 15, 2016 at 02:29:50PM -0800, anushasr wrote:
> > > > > From: Peter Antoine <peter.antoine@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
> > > > > v7: rebased. Rename I915_PARAM_HAS_HUC to I915_PARAM_HUC_STATUS.
> > > > > Remove intel_is_huc_valid() since it is used only in one place.
> > > > > Put the case of I915_PARAM_HAS_HUC() in the right place.
> > > > > 
> > > > > Signed-off-by: Peter Antoine <peter.antoine@intel.com>
> > > > > Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > > > > ---
> > > > >  drivers/gpu/drm/i915/i915_drv.c         | 4 ++++
> > > > >  drivers/gpu/drm/i915/intel_huc_loader.c | 1 -
> > > > >  include/uapi/drm/i915_drm.h             | 1 +
> > > > >  3 files changed, 5 insertions(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > > > > index 85a47c2..0bc016d 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;
> > > > >  
> > > > > @@ -315,6 +316,9 @@ static int i915_getparam(struct drm_device *dev, void *data,
> > > > >  	case I915_PARAM_MIN_EU_IN_POOL:
> > > > >  		value = INTEL_INFO(dev_priv)->sseu.min_eu_in_pool;
> > > > >  		break;
> > > > > +	case I915_PARAM_HUC_STATUS:
> > > > > +		value = I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED;
> > > > 
> > > > Same question as last time: does the device need to be awake? We know is
> > > > one of the GT power wells, so presumably we need an rpm_get/rpm_put as
> > > > well to access the register.
> > > > -Chris
> > > 
> > > I get:
> > > 
> > > [ 1588.570174] [drm:i915_huc_load_status_info [i915]] HUC_STATUS2 PRE  24704
> > > [ 1588.571285] [drm:intel_runtime_suspend [i915]] Suspending device
> > > [ 1588.575768] [drm:intel_runtime_suspend [i915]] Device suspended
> > > [ 1588.577156] [drm:i915_huc_load_status_info [i915]] HUC_STATUS2 POST 24704
> > > [ 1588.578259] [drm:intel_runtime_resume [i915]] Resuming device
> > > 
> > > consistently from:
> > > 
> > > value = I915_READ(HUC_STATUS2);
> > > DRM_DEBUG_DRIVER("HUC_STATUS2 PRE  %d\n", value);
> > > i915_pm_ops.runtime_suspend(dev_priv->drm.dev);
> > > 
> > > value = I915_READ(HUC_STATUS2);
> > > DRM_DEBUG_DRIVER("HUC_STATUS2 POST %d\n", value);
> > > i915_pm_ops.runtime_resume(dev_priv->drm.dev);
> > 
> > Also do the test with i915.mmio_debug=9999
> > -Chris
> 
> Same effect. Works.

Ok, then just mark up that we don't need rpm here so that we don't freak
out in future scans for mmio access outside of rpm.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 8/8] drm/i915/get_params: Add HuC status to getparams

[Srivatsa, Anusha]

>-----Original Message-----
>From: Chris Wilson [mailto:chris@chris-wilson.co.uk]
>Sent: Friday, December 16, 2016 8:31 AM
>To: Hiler, Arkadiusz <arkadiusz.hiler@intel.com>
>Cc: Srivatsa, Anusha <anusha.srivatsa@intel.com>; intel-
>gfx@lists.freedesktop.org
>Subject: Re: [Intel-gfx] [PATCH 8/8] drm/i915/get_params: Add HuC status to
>getparams
>
>On Fri, Dec 16, 2016 at 05:21:38PM +0100, Arkadiusz Hiler wrote:
>> On Fri, Dec 16, 2016 at 04:12:36PM +0000, Chris Wilson wrote:
>> > On Fri, Dec 16, 2016 at 03:43:46PM +0100, Arkadiusz Hiler wrote:
>> > > On Thu, Dec 15, 2016 at 10:42:53PM +0000, Chris Wilson wrote:
>> > > > On Thu, Dec 15, 2016 at 02:29:50PM -0800, anushasr wrote:
>> > > > > From: Peter Antoine <peter.antoine@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
>> > > > > v7: rebased. Rename I915_PARAM_HAS_HUC to
>I915_PARAM_HUC_STATUS.
>> > > > > Remove intel_is_huc_valid() since it is used only in one place.
>> > > > > Put the case of I915_PARAM_HAS_HUC() in the right place.
>> > > > >
>> > > > > Signed-off-by: Peter Antoine <peter.antoine@intel.com>
>> > > > > Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
>> > > > > ---
>> > > > >  drivers/gpu/drm/i915/i915_drv.c         | 4 ++++
>> > > > >  drivers/gpu/drm/i915/intel_huc_loader.c | 1 -
>> > > > >  include/uapi/drm/i915_drm.h             | 1 +
>> > > > >  3 files changed, 5 insertions(+), 1 deletion(-)
>> > > > >
>> > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c
>> > > > > b/drivers/gpu/drm/i915/i915_drv.c index 85a47c2..0bc016d
>> > > > > 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;
>> > > > >
>> > > > > @@ -315,6 +316,9 @@ static int i915_getparam(struct drm_device
>*dev, void *data,
>> > > > >  	case I915_PARAM_MIN_EU_IN_POOL:
>> > > > >  		value = INTEL_INFO(dev_priv)->sseu.min_eu_in_pool;
>> > > > >  		break;
>> > > > > +	case I915_PARAM_HUC_STATUS:
>> > > > > +		value = I915_READ(HUC_STATUS2) &
>HUC_FW_VERIFIED;
>> > > >
>> > > > Same question as last time: does the device need to be awake? We
>> > > > know is one of the GT power wells, so presumably we need an
>> > > > rpm_get/rpm_put as well to access the register.
>> > > > -Chris
>> > >
>> > > I get:
>> > >
>> > > [ 1588.570174] [drm:i915_huc_load_status_info [i915]] HUC_STATUS2
>> > > PRE  24704 [ 1588.571285] [drm:intel_runtime_suspend [i915]]
>> > > Suspending device [ 1588.575768] [drm:intel_runtime_suspend
>> > > [i915]] Device suspended [ 1588.577156]
>> > > [drm:i915_huc_load_status_info [i915]] HUC_STATUS2 POST 24704 [
>> > > 1588.578259] [drm:intel_runtime_resume [i915]] Resuming device
>> > >
>> > > consistently from:
>> > >
>> > > value = I915_READ(HUC_STATUS2);
>> > > DRM_DEBUG_DRIVER("HUC_STATUS2 PRE  %d\n", value);
>> > > i915_pm_ops.runtime_suspend(dev_priv->drm.dev);
>> > >
>> > > value = I915_READ(HUC_STATUS2);
>> > > DRM_DEBUG_DRIVER("HUC_STATUS2 POST %d\n", value);
>> > > i915_pm_ops.runtime_resume(dev_priv->drm.dev);
>> >
>> > Also do the test with i915.mmio_debug=9999 -Chris
>>
>> Same effect. Works.
Thanks Arek for confirming.

>Ok, then just mark up that we don't need rpm here so that we don't freak out in
>future scans for mmio access outside of rpm.
>-Chris
Chris, v2 as changed by Tvrtko suggests that forcewakes are removed since the register is force waken. Are you suggesting that adding that rpm not  being required in the commit message will make things much more clearer?

Anusha
>
>--
>Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 8/8] drm/i915/get_params: Add HuC status to getparams

[Chris Wilson]

On Fri, Dec 16, 2016 at 06:31:46PM +0000, Srivatsa, Anusha wrote:
> 
> 
> >-----Original Message-----
> >From: Chris Wilson [mailto:chris@chris-wilson.co.uk]
> >Sent: Friday, December 16, 2016 8:31 AM
> >To: Hiler, Arkadiusz <arkadiusz.hiler@intel.com>
> >Cc: Srivatsa, Anusha <anusha.srivatsa@intel.com>; intel-
> >gfx@lists.freedesktop.org
> >Subject: Re: [Intel-gfx] [PATCH 8/8] drm/i915/get_params: Add HuC status to
> >getparams
> >
> >On Fri, Dec 16, 2016 at 05:21:38PM +0100, Arkadiusz Hiler wrote:
> >> On Fri, Dec 16, 2016 at 04:12:36PM +0000, Chris Wilson wrote:
> >> > On Fri, Dec 16, 2016 at 03:43:46PM +0100, Arkadiusz Hiler wrote:
> >> > > On Thu, Dec 15, 2016 at 10:42:53PM +0000, Chris Wilson wrote:
> >> > > > On Thu, Dec 15, 2016 at 02:29:50PM -0800, anushasr wrote:
> >> > > > > From: Peter Antoine <peter.antoine@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
> >> > > > > v7: rebased. Rename I915_PARAM_HAS_HUC to
> >I915_PARAM_HUC_STATUS.
> >> > > > > Remove intel_is_huc_valid() since it is used only in one place.
> >> > > > > Put the case of I915_PARAM_HAS_HUC() in the right place.
> >> > > > >
> >> > > > > Signed-off-by: Peter Antoine <peter.antoine@intel.com>
> >> > > > > Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> >> > > > > ---
> >> > > > >  drivers/gpu/drm/i915/i915_drv.c         | 4 ++++
> >> > > > >  drivers/gpu/drm/i915/intel_huc_loader.c | 1 -
> >> > > > >  include/uapi/drm/i915_drm.h             | 1 +
> >> > > > >  3 files changed, 5 insertions(+), 1 deletion(-)
> >> > > > >
> >> > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c
> >> > > > > b/drivers/gpu/drm/i915/i915_drv.c index 85a47c2..0bc016d
> >> > > > > 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;
> >> > > > >
> >> > > > > @@ -315,6 +316,9 @@ static int i915_getparam(struct drm_device
> >*dev, void *data,
> >> > > > >  	case I915_PARAM_MIN_EU_IN_POOL:
> >> > > > >  		value = INTEL_INFO(dev_priv)->sseu.min_eu_in_pool;
> >> > > > >  		break;
> >> > > > > +	case I915_PARAM_HUC_STATUS:
> >> > > > > +		value = I915_READ(HUC_STATUS2) &
> >HUC_FW_VERIFIED;
> >> > > >
> >> > > > Same question as last time: does the device need to be awake? We
> >> > > > know is one of the GT power wells, so presumably we need an
> >> > > > rpm_get/rpm_put as well to access the register.
> >> > > > -Chris
> >> > >
> >> > > I get:
> >> > >
> >> > > [ 1588.570174] [drm:i915_huc_load_status_info [i915]] HUC_STATUS2
> >> > > PRE  24704 [ 1588.571285] [drm:intel_runtime_suspend [i915]]
> >> > > Suspending device [ 1588.575768] [drm:intel_runtime_suspend
> >> > > [i915]] Device suspended [ 1588.577156]
> >> > > [drm:i915_huc_load_status_info [i915]] HUC_STATUS2 POST 24704 [
> >> > > 1588.578259] [drm:intel_runtime_resume [i915]] Resuming device
> >> > >
> >> > > consistently from:
> >> > >
> >> > > value = I915_READ(HUC_STATUS2);
> >> > > DRM_DEBUG_DRIVER("HUC_STATUS2 PRE  %d\n", value);
> >> > > i915_pm_ops.runtime_suspend(dev_priv->drm.dev);
> >> > >
> >> > > value = I915_READ(HUC_STATUS2);
> >> > > DRM_DEBUG_DRIVER("HUC_STATUS2 POST %d\n", value);
> >> > > i915_pm_ops.runtime_resume(dev_priv->drm.dev);
> >> >
> >> > Also do the test with i915.mmio_debug=9999 -Chris
> >>
> >> Same effect. Works.
> Thanks Arek for confirming.
> 
> >Ok, then just mark up that we don't need rpm here so that we don't freak out in
> >future scans for mmio access outside of rpm.
> >-Chris
> Chris, v2 as changed by Tvrtko suggests that forcewakes are removed since the register is force waken. Are you suggesting that adding that rpm not  being required in the commit message will make things much more clearer?

No, if you are eschewing taking rpm around mmio access, I want that
commented upon in the code so that it is visible the next time we do an
audit for rpm abuse/misuse.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 8/8] drm/i915/get_params: Add HuC status to getparams

[Srivatsa, Anusha]

>-----Original Message-----
>From: Chris Wilson [mailto:chris@chris-wilson.co.uk]
>Sent: Friday, December 16, 2016 10:47 AM
>To: Srivatsa, Anusha <anusha.srivatsa@intel.com>
>Cc: Hiler, Arkadiusz <arkadiusz.hiler@intel.com>; intel-gfx@lists.freedesktop.org
>Subject: Re: [Intel-gfx] [PATCH 8/8] drm/i915/get_params: Add HuC status to
>getparams
>
>On Fri, Dec 16, 2016 at 06:31:46PM +0000, Srivatsa, Anusha wrote:
>>
>>
>> >-----Original Message-----
>> >From: Chris Wilson [mailto:chris@chris-wilson.co.uk]
>> >Sent: Friday, December 16, 2016 8:31 AM
>> >To: Hiler, Arkadiusz <arkadiusz.hiler@intel.com>
>> >Cc: Srivatsa, Anusha <anusha.srivatsa@intel.com>; intel-
>> >gfx@lists.freedesktop.org
>> >Subject: Re: [Intel-gfx] [PATCH 8/8] drm/i915/get_params: Add HuC
>> >status to getparams
>> >
>> >On Fri, Dec 16, 2016 at 05:21:38PM +0100, Arkadiusz Hiler wrote:
>> >> On Fri, Dec 16, 2016 at 04:12:36PM +0000, Chris Wilson wrote:
>> >> > On Fri, Dec 16, 2016 at 03:43:46PM +0100, Arkadiusz Hiler wrote:
>> >> > > On Thu, Dec 15, 2016 at 10:42:53PM +0000, Chris Wilson wrote:
>> >> > > > On Thu, Dec 15, 2016 at 02:29:50PM -0800, anushasr wrote:
>> >> > > > > From: Peter Antoine <peter.antoine@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
>> >> > > > > v7: rebased. Rename I915_PARAM_HAS_HUC to
>> >I915_PARAM_HUC_STATUS.
>> >> > > > > Remove intel_is_huc_valid() since it is used only in one place.
>> >> > > > > Put the case of I915_PARAM_HAS_HUC() in the right place.
>> >> > > > >
>> >> > > > > Signed-off-by: Peter Antoine <peter.antoine@intel.com>
>> >> > > > > Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
>> >> > > > > ---
>> >> > > > >  drivers/gpu/drm/i915/i915_drv.c         | 4 ++++
>> >> > > > >  drivers/gpu/drm/i915/intel_huc_loader.c | 1 -
>> >> > > > >  include/uapi/drm/i915_drm.h             | 1 +
>> >> > > > >  3 files changed, 5 insertions(+), 1 deletion(-)
>> >> > > > >
>> >> > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c
>> >> > > > > b/drivers/gpu/drm/i915/i915_drv.c index 85a47c2..0bc016d
>> >> > > > > 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;
>> >> > > > >
>> >> > > > > @@ -315,6 +316,9 @@ static int i915_getparam(struct
>> >> > > > > drm_device
>> >*dev, void *data,
>> >> > > > >  	case I915_PARAM_MIN_EU_IN_POOL:
>> >> > > > >  		value = INTEL_INFO(dev_priv)->sseu.min_eu_in_pool;
>> >> > > > >  		break;
>> >> > > > > +	case I915_PARAM_HUC_STATUS:
>> >> > > > > +		value = I915_READ(HUC_STATUS2) &
>> >HUC_FW_VERIFIED;
>> >> > > >
>> >> > > > Same question as last time: does the device need to be awake?
>> >> > > > We know is one of the GT power wells, so presumably we need
>> >> > > > an rpm_get/rpm_put as well to access the register.
>> >> > > > -Chris
>> >> > >
>> >> > > I get:
>> >> > >
>> >> > > [ 1588.570174] [drm:i915_huc_load_status_info [i915]]
>> >> > > HUC_STATUS2 PRE  24704 [ 1588.571285]
>> >> > > [drm:intel_runtime_suspend [i915]] Suspending device [
>> >> > > 1588.575768] [drm:intel_runtime_suspend [i915]] Device
>> >> > > suspended [ 1588.577156] [drm:i915_huc_load_status_info [i915]]
>> >> > > HUC_STATUS2 POST 24704 [ 1588.578259] [drm:intel_runtime_resume
>> >> > > [i915]] Resuming device
>> >> > >
>> >> > > consistently from:
>> >> > >
>> >> > > value = I915_READ(HUC_STATUS2);
>> >> > > DRM_DEBUG_DRIVER("HUC_STATUS2 PRE  %d\n", value);
>> >> > > i915_pm_ops.runtime_suspend(dev_priv->drm.dev);
>> >> > >
>> >> > > value = I915_READ(HUC_STATUS2);
>> >> > > DRM_DEBUG_DRIVER("HUC_STATUS2 POST %d\n", value);
>> >> > > i915_pm_ops.runtime_resume(dev_priv->drm.dev);
>> >> >
>> >> > Also do the test with i915.mmio_debug=9999 -Chris
>> >>
>> >> Same effect. Works.
>> Thanks Arek for confirming.
>>
>> >Ok, then just mark up that we don't need rpm here so that we don't
>> >freak out in future scans for mmio access outside of rpm.
>> >-Chris
>> Chris, v2 as changed by Tvrtko suggests that forcewakes are removed since the
>register is force waken. Are you suggesting that adding that rpm not  being
>required in the commit message will make things much more clearer?
>
>No, if you are eschewing taking rpm around mmio access, I want that
>commented upon in the code so that it is visible the next time we do an audit for
>rpm abuse/misuse.
Ah! Got it. Thanks.

Cheers,
Anusha
>-Chris
>
>--
>Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Anusha Srivatsa]

From: Peter Antoine <peter.antoine@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
v7: rebased. Rename I915_PARAM_HAS_HUC to I915_PARAM_HUC_STATUS.
Remove intel_is_huc_valid() since it is used only in one place.
Put the case of I915_PARAM_HAS_HUC() in the right place.
v8: rebased. Add a comment to specify that I915_READ(reg)
does not read garbage value. The register HUC_STATUS2 is force
woken and no rpm is needed.

Signed-off-by: Peter Antoine <peter.antoine@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c         | 7 +++++++
 drivers/gpu/drm/i915/intel_huc_loader.c | 1 -
 include/uapi/drm/i915_drm.h             | 1 +
 3 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 85a47c2..c4f0620 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;
 
@@ -315,6 +316,12 @@ static int i915_getparam(struct drm_device *dev, void *data,
 	case I915_PARAM_MIN_EU_IN_POOL:
 		value = INTEL_INFO(dev_priv)->sseu.min_eu_in_pool;
 		break;
+	case I915_PARAM_HUC_STATUS:
+		/* The register is already force-woken. We dont need
+		 * any rpm here
+		 */
+		value = I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED;
+		break;
 	case I915_PARAM_MMAP_GTT_VERSION:
 		/* Though we've started our numbering from 1, and so class all
 		 * earlier versions as 0, in effect their value is undefined as
diff --git a/drivers/gpu/drm/i915/intel_huc_loader.c b/drivers/gpu/drm/i915/intel_huc_loader.c
index 75f3dc5..dd42676 100644
--- a/drivers/gpu/drm/i915/intel_huc_loader.c
+++ b/drivers/gpu/drm/i915/intel_huc_loader.c
@@ -284,4 +284,3 @@ void intel_huc_fini(struct drm_device *dev)
 
 	huc_fw->fetch_status = INTEL_UC_FIRMWARE_NONE;
 }
-
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index da32c2f..57093b4 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -395,6 +395,7 @@ typedef struct drm_i915_irq_wait {
  * priorities and the driver will attempt to execute batches in priority order.
  */
 #define I915_PARAM_HAS_SCHEDULER	 41
+#define I915_PARAM_HUC_STATUS		 42
 
 typedef struct drm_i915_getparam {
 	__s32 param;
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 8/8] drm/i915/get_params: Add HuC status to getparams

[Arkadiusz Hiler]

On Thu, Dec 22, 2016 at 03:12:24PM -0800, Anusha Srivatsa wrote:
> From: Peter Antoine <peter.antoine@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
> v7: rebased. Rename I915_PARAM_HAS_HUC to I915_PARAM_HUC_STATUS.
> Remove intel_is_huc_valid() since it is used only in one place.
> Put the case of I915_PARAM_HAS_HUC() in the right place.
> v8: rebased. Add a comment to specify that I915_READ(reg)
> does not read garbage value. The register HUC_STATUS2 is force
> woken and no rpm is needed.
> 
> Signed-off-by: Peter Antoine <peter.antoine@intel.com>

Where is your s-o-b?

other than that:
Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>

-- 
Cheers,
Arek
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Anusha Srivatsa]

From: Peter Antoine <peter.antoine@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
v7: rebased. Rename I915_PARAM_HAS_HUC to I915_PARAM_HUC_STATUS.
Remove intel_is_huc_valid() since it is used only in one place.
Put the case of I915_PARAM_HAS_HUC() in the right place.
v8: rebased. Add a comment to specify that I915_READ(reg)
does not read garbage value. The register HUC_STATUS2 is force
woken and no rpm is needed.
v9: rebased.

Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
Signed-off-by: Peter Antoine <peter.antoine@intel.com>
Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c         | 7 +++++++
 drivers/gpu/drm/i915/intel_huc_loader.c | 1 -
 include/uapi/drm/i915_drm.h             | 1 +
 3 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index c9f71e0..723442c 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;
 
@@ -315,6 +316,12 @@ static int i915_getparam(struct drm_device *dev, void *data,
 	case I915_PARAM_MIN_EU_IN_POOL:
 		value = INTEL_INFO(dev_priv)->sseu.min_eu_in_pool;
 		break;
+	case I915_PARAM_HUC_STATUS:
+		/* The register is already force-woken. We dont need
+		 * any rpm here
+		 */
+		value = I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED;
+		break;
 	case I915_PARAM_MMAP_GTT_VERSION:
 		/* Though we've started our numbering from 1, and so class all
 		 * earlier versions as 0, in effect their value is undefined as
diff --git a/drivers/gpu/drm/i915/intel_huc_loader.c b/drivers/gpu/drm/i915/intel_huc_loader.c
index 051e7ba..d0a7547 100644
--- a/drivers/gpu/drm/i915/intel_huc_loader.c
+++ b/drivers/gpu/drm/i915/intel_huc_loader.c
@@ -281,4 +281,3 @@ void intel_huc_fini(struct drm_device *dev)
 
 	huc_fw->fetch_status = INTEL_UC_FIRMWARE_NONE;
 }
-
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index da32c2f..57093b4 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -395,6 +395,7 @@ typedef struct drm_i915_irq_wait {
  * priorities and the driver will attempt to execute batches in priority order.
  */
 #define I915_PARAM_HAS_SCHEDULER	 41
+#define I915_PARAM_HUC_STATUS		 42
 
 typedef struct drm_i915_getparam {
 	__s32 param;
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Anusha Srivatsa]

From: Peter Antoine <peter.antoine@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
v7: rebased. Rename I915_PARAM_HAS_HUC to I915_PARAM_HUC_STATUS.
Remove intel_is_huc_valid() since it is used only in one place.
Put the case of I915_PARAM_HAS_HUC() in the right place.
v8: rebased. Add a comment to specify that I915_READ(reg)
does not read garbage value. The register HUC_STATUS2 is force
woken and no rpm is needed.
v9: rebased.

Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
Signed-off-by: Peter Antoine <peter.antoine@intel.com>
Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c         | 7 +++++++
 drivers/gpu/drm/i915/intel_huc_loader.c | 1 -
 include/uapi/drm/i915_drm.h             | 1 +
 3 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index c9f71e0..723442c 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;
 
@@ -315,6 +316,12 @@ static int i915_getparam(struct drm_device *dev, void *data,
 	case I915_PARAM_MIN_EU_IN_POOL:
 		value = INTEL_INFO(dev_priv)->sseu.min_eu_in_pool;
 		break;
+	case I915_PARAM_HUC_STATUS:
+		/* The register is already force-woken. We dont need
+		 * any rpm here
+		 */
+		value = I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED;
+		break;
 	case I915_PARAM_MMAP_GTT_VERSION:
 		/* Though we've started our numbering from 1, and so class all
 		 * earlier versions as 0, in effect their value is undefined as
diff --git a/drivers/gpu/drm/i915/intel_huc_loader.c b/drivers/gpu/drm/i915/intel_huc_loader.c
index 57fc48b..af75d1d 100644
--- a/drivers/gpu/drm/i915/intel_huc_loader.c
+++ b/drivers/gpu/drm/i915/intel_huc_loader.c
@@ -281,4 +281,3 @@ void intel_huc_fini(struct drm_device *dev)
 
 	huc_fw->fetch_status = INTEL_UC_FIRMWARE_NONE;
 }
-
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index da32c2f..57093b4 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -395,6 +395,7 @@ typedef struct drm_i915_irq_wait {
  * priorities and the driver will attempt to execute batches in priority order.
  */
 #define I915_PARAM_HAS_SCHEDULER	 41
+#define I915_PARAM_HUC_STATUS		 42
 
 typedef struct drm_i915_getparam {
 	__s32 param;
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Anusha Srivatsa]

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)
v3: rebased on top of drm-tip. Removed any reference to intel_huc.h
v4: rebased. Rename I915_PARAM_HAS_HUC to I915_PARAM_HUC_STATUS.
Remove intel_is_huc_valid() since it is used only in one place.
Put the case of I915_PARAM_HAS_HUC() in the right place.
v5: rebased. Add a comment to specify that I915_READ(reg)
does not read garbage value. The register HUC_STATUS2 is force
woken and no rpm is needed.

Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
Signed-off-by: Peter Antoine <peter.antoine@intel.com>
Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c | 7 +++++++
 include/uapi/drm/i915_drm.h     | 1 +
 2 files changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index d7a0b49..49a927a 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;
 
@@ -315,6 +316,12 @@ static int i915_getparam(struct drm_device *dev, void *data,
 	case I915_PARAM_MIN_EU_IN_POOL:
 		value = INTEL_INFO(dev_priv)->sseu.min_eu_in_pool;
 		break;
+	case I915_PARAM_HUC_STATUS:
+		/* The register is already force-woken. We dont need
+		 * any rpm here
+		 */
+		value = I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED;
+		break;
 	case I915_PARAM_MMAP_GTT_VERSION:
 		/* Though we've started our numbering from 1, and so class all
 		 * earlier versions as 0, in effect their value is undefined as
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index da32c2f..57093b4 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -395,6 +395,7 @@ typedef struct drm_i915_irq_wait {
  * priorities and the driver will attempt to execute batches in priority order.
  */
 #define I915_PARAM_HAS_SCHEDULER	 41
+#define I915_PARAM_HUC_STATUS		 42
 
 typedef struct drm_i915_getparam {
 	__s32 param;
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Anusha Srivatsa]

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)
v3: rebased on top of drm-tip. Removed any reference to intel_huc.h
v4: rebased. Rename I915_PARAM_HAS_HUC to I915_PARAM_HUC_STATUS.
Remove intel_is_huc_valid() since it is used only in one place.
Put the case of I915_PARAM_HAS_HUC() in the right place.
v5: rebased. Add a comment to specify that I915_READ(reg)
does not read garbage value. The register HUC_STATUS2 is force
woken and no rpm is needed.

Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
Signed-off-by: Peter Antoine <peter.antoine@intel.com>
Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c | 7 +++++++
 include/uapi/drm/i915_drm.h     | 1 +
 2 files changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index d7a0b49..49a927a 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;
 
@@ -315,6 +316,12 @@ static int i915_getparam(struct drm_device *dev, void *data,
 	case I915_PARAM_MIN_EU_IN_POOL:
 		value = INTEL_INFO(dev_priv)->sseu.min_eu_in_pool;
 		break;
+	case I915_PARAM_HUC_STATUS:
+		/* The register is already force-woken. We dont need
+		 * any rpm here
+		 */
+		value = I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED;
+		break;
 	case I915_PARAM_MMAP_GTT_VERSION:
 		/* Though we've started our numbering from 1, and so class all
 		 * earlier versions as 0, in effect their value is undefined as
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index da32c2f..57093b4 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -395,6 +395,7 @@ typedef struct drm_i915_irq_wait {
  * priorities and the driver will attempt to execute batches in priority order.
  */
 #define I915_PARAM_HAS_SCHEDULER	 41
+#define I915_PARAM_HUC_STATUS		 42
 
 typedef struct drm_i915_getparam {
 	__s32 param;
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Anusha Srivatsa]

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)
v3: rebased on top of drm-tip. Removed any reference to intel_huc.h
v4: rebased. Rename I915_PARAM_HAS_HUC to I915_PARAM_HUC_STATUS.
Remove intel_is_huc_valid() since it is used only in one place.
Put the case of I915_PARAM_HAS_HUC() in the right place.
v5: rebased. Add a comment to specify that I915_READ(reg)
does not read garbage value. The register HUC_STATUS2 is force
woken and no rpm is needed.

Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
Signed-off-by: Peter Antoine <peter.antoine@intel.com>
Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c | 7 +++++++
 include/uapi/drm/i915_drm.h     | 1 +
 2 files changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index d7a0b49..49a927a 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;
 
@@ -315,6 +316,12 @@ static int i915_getparam(struct drm_device *dev, void *data,
 	case I915_PARAM_MIN_EU_IN_POOL:
 		value = INTEL_INFO(dev_priv)->sseu.min_eu_in_pool;
 		break;
+	case I915_PARAM_HUC_STATUS:
+		/* The register is already force-woken. We dont need
+		 * any rpm here
+		 */
+		value = I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED;
+		break;
 	case I915_PARAM_MMAP_GTT_VERSION:
 		/* Though we've started our numbering from 1, and so class all
 		 * earlier versions as 0, in effect their value is undefined as
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index da32c2f..57093b4 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -395,6 +395,7 @@ typedef struct drm_i915_irq_wait {
  * priorities and the driver will attempt to execute batches in priority order.
  */
 #define I915_PARAM_HAS_SCHEDULER	 41
+#define I915_PARAM_HUC_STATUS		 42
 
 typedef struct drm_i915_getparam {
 	__s32 param;
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx