[PATCH] drm/i915/guc: Take enable_guc_loading check out of GEM core code

[PATCH] drm/i915/guc: Take enable_guc_loading check out of GEM core code

[Oscar Mateo]

The should happen as soon as possible, but always within the logic that
depends on it (and not interrupting the top-level driver control flow).

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c |  3 +--
 drivers/gpu/drm/i915/i915_gem.c | 10 ++++------
 drivers/gpu/drm/i915/intel_uc.c |  6 ++++++
 3 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index d0524fb..c616b4e 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -549,8 +549,7 @@ static bool i915_switcheroo_can_switch(struct pci_dev *pdev)
 static void i915_gem_fini(struct drm_i915_private *dev_priv)
 {
 	mutex_lock(&dev_priv->drm.struct_mutex);
-	if (i915.enable_guc_loading)
-		intel_uc_fini_hw(dev_priv);
+	intel_uc_fini_hw(dev_priv);
 	i915_gem_cleanup_engines(dev_priv);
 	i915_gem_context_fini(dev_priv);
 	mutex_unlock(&dev_priv->drm.struct_mutex);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 84ea249..2709be9 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4611,12 +4611,10 @@ int i915_gem_init_hw(struct drm_i915_private *dev_priv)
 
 	intel_mocs_init_l3cc_table(dev_priv);
 
-	if (i915.enable_guc_loading) {
-		/* We can't enable contexts until all firmware is loaded */
-		ret = intel_uc_init_hw(dev_priv);
-		if (ret)
-			goto out;
-	}
+	/* We can't enable contexts until all firmware is loaded */
+	ret = intel_uc_init_hw(dev_priv);
+	if (ret)
+		goto out;
 
 out:
 	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index c767dc3..1965322 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -254,6 +254,9 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
 {
 	int ret, attempts;
 
+	if (!i915.enable_guc_loading)
+		return 0;
+
 	gen9_reset_guc_interrupts(dev_priv);
 
 	/* We need to notify the guc whenever we change the GGTT */
@@ -343,6 +346,9 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
 
 void intel_uc_fini_hw(struct drm_i915_private *dev_priv)
 {
+	if (!i915.enable_guc_loading)
+		return;
+
 	if (i915.enable_guc_submission) {
 		i915_guc_submission_disable(dev_priv);
 		gen9_disable_guc_interrupts(dev_priv);
-- 
1.9.1

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

Re: [PATCH] drm/i915/guc: Take enable_guc_loading check out of GEM core code

[Oscar Mateo]

I wanted to get this out of the way as soon as possible, but I'll follow 
shortly with a proposal to remove enable_guc_loading as a module parameter.

Thanks,

Oscar


On 03/28/2017 09:53 AM, Oscar Mateo wrote:
> The should happen as soon as possible, but always within the logic that
> depends on it (and not interrupting the top-level driver control flow).
>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_drv.c |  3 +--
>   drivers/gpu/drm/i915/i915_gem.c | 10 ++++------
>   drivers/gpu/drm/i915/intel_uc.c |  6 ++++++
>   3 files changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index d0524fb..c616b4e 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -549,8 +549,7 @@ static bool i915_switcheroo_can_switch(struct pci_dev *pdev)
>   static void i915_gem_fini(struct drm_i915_private *dev_priv)
>   {
>   	mutex_lock(&dev_priv->drm.struct_mutex);
> -	if (i915.enable_guc_loading)
> -		intel_uc_fini_hw(dev_priv);
> +	intel_uc_fini_hw(dev_priv);
>   	i915_gem_cleanup_engines(dev_priv);
>   	i915_gem_context_fini(dev_priv);
>   	mutex_unlock(&dev_priv->drm.struct_mutex);
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 84ea249..2709be9 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4611,12 +4611,10 @@ int i915_gem_init_hw(struct drm_i915_private *dev_priv)
>   
>   	intel_mocs_init_l3cc_table(dev_priv);
>   
> -	if (i915.enable_guc_loading) {
> -		/* We can't enable contexts until all firmware is loaded */
> -		ret = intel_uc_init_hw(dev_priv);
> -		if (ret)
> -			goto out;
> -	}
> +	/* We can't enable contexts until all firmware is loaded */
> +	ret = intel_uc_init_hw(dev_priv);
> +	if (ret)
> +		goto out;
>   
>   out:
>   	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
> diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
> index c767dc3..1965322 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -254,6 +254,9 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
>   {
>   	int ret, attempts;
>   
> +	if (!i915.enable_guc_loading)
> +		return 0;
> +
>   	gen9_reset_guc_interrupts(dev_priv);
>   
>   	/* We need to notify the guc whenever we change the GGTT */
> @@ -343,6 +346,9 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
>   
>   void intel_uc_fini_hw(struct drm_i915_private *dev_priv)
>   {
> +	if (!i915.enable_guc_loading)
> +		return;
> +
>   	if (i915.enable_guc_submission) {
>   		i915_guc_submission_disable(dev_priv);
>   		gen9_disable_guc_interrupts(dev_priv);

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

✓ Fi.CI.BAT: success for drm/i915/guc: Take enable_guc_loading check out of GEM core code

[Patchwork]

== Series Details ==

Series: drm/i915/guc: Take enable_guc_loading check out of GEM core code
URL   : https://patchwork.freedesktop.org/series/22041/
State : success

== Summary ==

Series 22041v1 drm/i915/guc: Take enable_guc_loading check out of GEM core code
https://patchwork.freedesktop.org/api/1.0/series/22041/revisions/1/mbox/

Test gem_exec_suspend:
        Subgroup basic-s3:
                incomplete -> PASS       (fi-skl-6260u)
        Subgroup basic-s4-devices:
                dmesg-warn -> PASS       (fi-kbl-7560u) fdo#100428
Test prime_vgem:
        Subgroup basic-fence-flip:
                pass       -> DMESG-WARN (fi-byt-n2820) fdo#100094

fdo#100428 https://bugs.freedesktop.org/show_bug.cgi?id=100428
fdo#100094 https://bugs.freedesktop.org/show_bug.cgi?id=100094

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time: 467s
fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time: 458s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time: 540s
fi-bxt-t5700     total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20  time: 573s
fi-byt-j1900     total:278  pass:251  dwarn:0   dfail:0   fail:0   skip:27  time: 506s
fi-byt-n2820     total:278  pass:246  dwarn:1   dfail:0   fail:0   skip:31  time: 516s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 436s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 445s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time: 439s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 521s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 520s
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 490s
fi-kbl-7560u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 590s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 487s
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  time: 601s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time: 484s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 521s
fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time: 462s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time: 548s
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time: 422s
fi-bsw-n3050 failed to collect. IGT log at Patchwork_4331/fi-bsw-n3050/igt.log

dabd992961047cf26698036f563aa86a083284ac drm-tip: 2017y-03m-28d-15h-25m-54s UTC integration manifest
2cbd2dc drm/i915/guc: Take enable_guc_loading check out of GEM core code

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4331/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✓ Fi.CI.BAT: success for drm/i915/guc: Take enable_guc_loading check out of GEM core code

[Joonas Lahtinen]

Pushing this, thanks for the patch and review.

On ke, 2017-03-29 at 00:12 +0000, Patchwork wrote:
> == Series Details ==
> 
> Series: drm/i915/guc: Take enable_guc_loading check out of GEM core code
> URL   : https://patchwork.freedesktop.org/series/22041/
> State : success
> 
> == Summary ==
> 
> Series 22041v1 drm/i915/guc: Take enable_guc_loading check out of GEM core code
> https://patchwork.freedesktop.org/api/1.0/series/22041/revisions/1/mbox/
> 
> Test gem_exec_suspend:
>         Subgroup basic-s3:
>                 incomplete -> PASS       (fi-skl-6260u)
>         Subgroup basic-s4-devices:
>                 dmesg-warn -> PASS       (fi-kbl-7560u) fdo#100428
> Test prime_vgem:
>         Subgroup basic-fence-flip:
>                 pass       -> DMESG-WARN (fi-byt-n2820) fdo#100094
> 
> fdo#100428 https://bugs.freedesktop.org/show_bug.cgi?id=100428
> fdo#100094 https://bugs.freedesktop.org/show_bug.cgi?id=100094
> 
> fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time: 467s
> fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time: 458s
> fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time: 540s
> fi-bxt-t5700     total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20  time: 573s
> fi-byt-j1900     total:278  pass:251  dwarn:0   dfail:0   fail:0   skip:27  time: 506s
> fi-byt-n2820     total:278  pass:246  dwarn:1   dfail:0   fail:0   skip:31  time: 516s
> fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 436s
> fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 445s
> fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time: 439s
> fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 521s
> fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 520s
> fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 490s
> fi-kbl-7560u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 590s
> fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 487s
> fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  time: 601s
> fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time: 484s
> fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 521s
> fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time: 462s
> fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time: 548s
> fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time: 422s
> fi-bsw-n3050 failed to collect. IGT log at Patchwork_4331/fi-bsw-n3050/igt.log
> 
> dabd992961047cf26698036f563aa86a083284ac drm-tip: 2017y-03m-28d-15h-25m-54s UTC integration manifest
> 2cbd2dc drm/i915/guc: Take enable_guc_loading check out of GEM core code
> 
> == Logs ==
> 
> For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4331/
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/guc: Take enable_guc_loading check out of GEM core code

[Chris Wilson]

On Tue, Mar 28, 2017 at 09:53:47AM -0700, Oscar Mateo wrote:
> The should happen as soon as possible, but always within the logic that
> depends on it (and not interrupting the top-level driver control flow).
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>

>  void intel_uc_fini_hw(struct drm_i915_private *dev_priv)
>  {
> +	if (!i915.enable_guc_loading)
> +		return;
> +
>  	if (i915.enable_guc_submission) {
>  		i915_guc_submission_disable(dev_priv);
>  		gen9_disable_guc_interrupts(dev_priv);

With the understanding that this becomes more interesting in the future
:)

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-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