diff mbox

drm/i915: Make intel_uc_sanitize_options() more robust

Message ID 20170315133741.150420-1-michal.wajdeczko@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Michal Wajdeczko March 15, 2017, 1:37 p.m. UTC
After negative guc fw selection we could leave guc
submission flag still turned on. Reorder some checks
to cover this case. While here, fix info message and
return early if there is no Guc.

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_uc.c | 29 +++++++++++++++++------------
 1 file changed, 17 insertions(+), 12 deletions(-)

Comments

Arkadiusz Hiler March 15, 2017, 1:58 p.m. UTC | #1
On Wed, Mar 15, 2017 at 01:37:41PM +0000, Michal Wajdeczko wrote:
> After negative guc fw selection we could leave guc
> submission flag still turned on. Reorder some checks
> to cover this case. While here, fix info message and
> return early if there is no Guc.
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Tvrtko Ursulin March 16, 2017, 8:59 a.m. UTC | #2
On 15/03/2017 13:37, Michal Wajdeczko wrote:
> After negative guc fw selection we could leave guc
> submission flag still turned on. Reorder some checks
> to cover this case. While here, fix info message and
> return early if there is no Guc.
>
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_uc.c | 29 +++++++++++++++++------------
>  1 file changed, 17 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
> index c31f05a..54c5aff 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -50,23 +50,20 @@ static int __intel_uc_reset_hw(struct drm_i915_private *dev_priv)
>  void intel_uc_sanitize_options(struct drm_i915_private *dev_priv)
>  {
>  	if (!HAS_GUC(dev_priv)) {
> -		if (i915.enable_guc_loading > 0)
> -			DRM_INFO("Ignoring GuC options, no hardware");
> +		if (i915.enable_guc_loading > 0 ||
> +			i915.enable_guc_submission > 0)
> +			DRM_INFO("Ignoring GuC options, no hardware\n");

Alignment is incorrect.

I've fixed it up while pushing.

Regards,

Tvrtko

>
>  		i915.enable_guc_loading = 0;
>  		i915.enable_guc_submission = 0;
> -	} else {
> -		/* A negative value means "use platform default" */
> -		if (i915.enable_guc_loading < 0)
> -			i915.enable_guc_loading = HAS_GUC_UCODE(dev_priv);
> -		if (i915.enable_guc_submission < 0)
> -			i915.enable_guc_submission = HAS_GUC_SCHED(dev_priv);
> -
> -		/* Can't enable guc submission without guc loaded */
> -		if (!i915.enable_guc_loading)
> -			i915.enable_guc_submission = 0;
> +		return;
>  	}
>
> +	/* A negative value means "use platform default" */
> +	if (i915.enable_guc_loading < 0)
> +		i915.enable_guc_loading = HAS_GUC_UCODE(dev_priv);
> +
> +	/* Verify firmware version */
>  	if (i915.enable_guc_loading) {
>  		if (HAS_HUC_UCODE(dev_priv))
>  			intel_huc_select_fw(&dev_priv->huc);
> @@ -74,6 +71,14 @@ void intel_uc_sanitize_options(struct drm_i915_private *dev_priv)
>  		if (intel_guc_select_fw(&dev_priv->guc))
>  			i915.enable_guc_loading = 0;
>  	}
> +
> +	/* Can't enable guc submission without guc loaded */
> +	if (!i915.enable_guc_loading)
> +		i915.enable_guc_submission = 0;
> +
> +	/* A negative value means "use platform default" */
> +	if (i915.enable_guc_submission < 0)
> +		i915.enable_guc_submission = HAS_GUC_SCHED(dev_priv);
>  }
>
>  void intel_uc_init_early(struct drm_i915_private *dev_priv)
>
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index c31f05a..54c5aff 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -50,23 +50,20 @@  static int __intel_uc_reset_hw(struct drm_i915_private *dev_priv)
 void intel_uc_sanitize_options(struct drm_i915_private *dev_priv)
 {
 	if (!HAS_GUC(dev_priv)) {
-		if (i915.enable_guc_loading > 0)
-			DRM_INFO("Ignoring GuC options, no hardware");
+		if (i915.enable_guc_loading > 0 ||
+			i915.enable_guc_submission > 0)
+			DRM_INFO("Ignoring GuC options, no hardware\n");
 
 		i915.enable_guc_loading = 0;
 		i915.enable_guc_submission = 0;
-	} else {
-		/* A negative value means "use platform default" */
-		if (i915.enable_guc_loading < 0)
-			i915.enable_guc_loading = HAS_GUC_UCODE(dev_priv);
-		if (i915.enable_guc_submission < 0)
-			i915.enable_guc_submission = HAS_GUC_SCHED(dev_priv);
-
-		/* Can't enable guc submission without guc loaded */
-		if (!i915.enable_guc_loading)
-			i915.enable_guc_submission = 0;
+		return;
 	}
 
+	/* A negative value means "use platform default" */
+	if (i915.enable_guc_loading < 0)
+		i915.enable_guc_loading = HAS_GUC_UCODE(dev_priv);
+
+	/* Verify firmware version */
 	if (i915.enable_guc_loading) {
 		if (HAS_HUC_UCODE(dev_priv))
 			intel_huc_select_fw(&dev_priv->huc);
@@ -74,6 +71,14 @@  void intel_uc_sanitize_options(struct drm_i915_private *dev_priv)
 		if (intel_guc_select_fw(&dev_priv->guc))
 			i915.enable_guc_loading = 0;
 	}
+
+	/* Can't enable guc submission without guc loaded */
+	if (!i915.enable_guc_loading)
+		i915.enable_guc_submission = 0;
+
+	/* A negative value means "use platform default" */
+	if (i915.enable_guc_submission < 0)
+		i915.enable_guc_submission = HAS_GUC_SCHED(dev_priv);
 }
 
 void intel_uc_init_early(struct drm_i915_private *dev_priv)