[PATCH 1/1] drm/i915: Move i915_gem_restore_fences to i915_gem_resume

[PATCH 1/1] drm/i915: Move i915_gem_restore_fences to i915_gem_resume

[Sagar Arun Kamble]

i915_gem_restore_fences is GEM resumption task hence it is moved to
i915_gem_resume from i915_restore_state.

Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Michał Winiarski <michal.winiarski@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c     | 1 +
 drivers/gpu/drm/i915/i915_suspend.c | 2 --
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 73eeb6b..ab8c694 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4595,6 +4595,7 @@ void i915_gem_resume(struct drm_i915_private *dev_priv)
 
 	mutex_lock(&dev->struct_mutex);
 	i915_gem_restore_gtt_mappings(dev_priv);
+	i915_gem_restore_fences(dev_priv);
 
 	/* As we didn't flush the kernel context before suspend, we cannot
 	 * guarantee that the context image is complete. So let's just reset
diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c
index 5c86925a..8f3aa4d 100644
--- a/drivers/gpu/drm/i915/i915_suspend.c
+++ b/drivers/gpu/drm/i915/i915_suspend.c
@@ -108,8 +108,6 @@ int i915_restore_state(struct drm_i915_private *dev_priv)
 
 	mutex_lock(&dev_priv->drm.struct_mutex);
 
-	i915_gem_restore_fences(dev_priv);
-
 	if (IS_GEN4(dev_priv))
 		pci_write_config_word(pdev, GCDGMBUS,
 				      dev_priv->regfile.saveGCDGMBUS);
-- 
1.9.1

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

✗ Fi.CI.BAT: warning for series starting with [1/1] drm/i915: Move i915_gem_restore_fences to i915_gem_resume

[Patchwork]

== Series Details ==

Series: series starting with [1/1] drm/i915: Move i915_gem_restore_fences to i915_gem_resume
URL   : https://patchwork.freedesktop.org/series/31118/
State : warning

== Summary ==

Series 31118v1 series starting with [1/1] drm/i915: Move i915_gem_restore_fences to i915_gem_resume
https://patchwork.freedesktop.org/api/1.0/series/31118/revisions/1/mbox/

Test chamelium:
        Subgroup dp-crc-fast:
                fail       -> PASS       (fi-kbl-7500u) fdo#102514
Test kms_busy:
        Subgroup basic-flip-c:
                pass       -> DMESG-WARN (fi-bxt-dsi)
Test drv_module_reload:
        Subgroup basic-reload:
                pass       -> DMESG-WARN (fi-glk-1) fdo#102777 +1

fdo#102514 https://bugs.freedesktop.org/show_bug.cgi?id=102514
fdo#102777 https://bugs.freedesktop.org/show_bug.cgi?id=102777

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:442s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:472s
fi-blb-e6850     total:289  pass:224  dwarn:1   dfail:0   fail:0   skip:64  time:413s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:513s
fi-bwr-2160      total:289  pass:184  dwarn:0   dfail:0   fail:0   skip:105 time:280s
fi-bxt-dsi       total:289  pass:258  dwarn:1   dfail:0   fail:0   skip:30  time:492s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:509s
fi-byt-j1900     total:289  pass:254  dwarn:1   dfail:0   fail:0   skip:34  time:493s
fi-byt-n2820     total:289  pass:250  dwarn:1   dfail:0   fail:0   skip:38  time:481s
fi-cfl-s         total:289  pass:256  dwarn:1   dfail:0   fail:0   skip:32  time:571s
fi-cnl-y         total:289  pass:260  dwarn:1   dfail:0   fail:1   skip:27  time:635s
fi-elk-e7500     total:289  pass:230  dwarn:0   dfail:0   fail:0   skip:59  time:418s
fi-glk-1         total:289  pass:259  dwarn:1   dfail:0   fail:0   skip:29  time:566s
fi-hsw-4770      total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:424s
fi-hsw-4770r     total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:406s
fi-ilk-650       total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:428s
fi-ivb-3520m     total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:482s
fi-ivb-3770      total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:464s
fi-kbl-7500u     total:289  pass:264  dwarn:1   dfail:0   fail:0   skip:24  time:476s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:574s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:593s
fi-pnv-d510      total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:548s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:454s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:757s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:491s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:479s
fi-snb-2520m     total:289  pass:251  dwarn:0   dfail:0   fail:0   skip:38  time:561s
fi-snb-2600      total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:417s

0369ecdbb55495dd4671ad33d375f37d0707b629 drm-tip: 2017y-09m-28d-20h-01m-55s UTC integration manifest
c4a93dd537c3 drm/i915: Move i915_gem_restore_fences to i915_gem_resume

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5856/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/1] drm/i915: Move i915_gem_restore_fences to i915_gem_resume

[Michal Wajdeczko]

On Fri, 29 Sep 2017 06:58:36 +0200, Sagar Arun Kamble  
<sagar.a.kamble@intel.com> wrote:

> i915_gem_restore_fences is GEM resumption task hence it is moved to
> i915_gem_resume from i915_restore_state.
>
> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Michał Winiarski <michal.winiarski@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---

Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>

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

Re: [PATCH 1/1] drm/i915: Move i915_gem_restore_fences to i915_gem_resume

[Joonas Lahtinen]

On Fri, 2017-09-29 at 10:28 +0530, Sagar Arun Kamble wrote:
> i915_gem_restore_fences is GEM resumption task hence it is moved to
> i915_gem_resume from i915_restore_state.
> 
> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Michał Winiarski <michal.winiarski@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Yep, this is a more appropriate place. Locality to the display resume
was probably reason for the odd placement previously (it was added to
fix a bug of fences not being there for display resume).

Do we want to comment this requirement between i915_gem_resume and
i915_restore_state? Chris?

Anyway,

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Regards, Joonas

PS. My guess seems to be correct according to git blame (skipping one
rename in between) info.

> ---
>  drivers/gpu/drm/i915/i915_gem.c     | 1 +
>  drivers/gpu/drm/i915/i915_suspend.c | 2 --
>  2 files changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 73eeb6b..ab8c694 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4595,6 +4595,7 @@ void i915_gem_resume(struct drm_i915_private *dev_priv)
>  
>  	mutex_lock(&dev->struct_mutex);
>  	i915_gem_restore_gtt_mappings(dev_priv);
> +	i915_gem_restore_fences(dev_priv);
>  
>  	/* As we didn't flush the kernel context before suspend, we cannot
>  	 * guarantee that the context image is complete. So let's just reset
> diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c
> index 5c86925a..8f3aa4d 100644
> --- a/drivers/gpu/drm/i915/i915_suspend.c
> +++ b/drivers/gpu/drm/i915/i915_suspend.c
> @@ -108,8 +108,6 @@ int i915_restore_state(struct drm_i915_private *dev_priv)
>  
>  	mutex_lock(&dev_priv->drm.struct_mutex);
>  
> -	i915_gem_restore_fences(dev_priv);
> -
>  	if (IS_GEN4(dev_priv))
>  		pci_write_config_word(pdev, GCDGMBUS,
>  				      dev_priv->regfile.saveGCDGMBUS);
-- 
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 1/1] drm/i915: Move i915_gem_restore_fences to i915_gem_resume

[Chris Wilson]

Quoting Joonas Lahtinen (2017-09-29 08:08:31)
> On Fri, 2017-09-29 at 10:28 +0530, Sagar Arun Kamble wrote:
> > i915_gem_restore_fences is GEM resumption task hence it is moved to
> > i915_gem_resume from i915_restore_state.
> > 
> > Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> > Cc: Michał Winiarski <michal.winiarski@intel.com>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> 
> Yep, this is a more appropriate place. Locality to the display resume
> was probably reason for the odd placement previously (it was added to
> fix a bug of fences not being there for display resume).
> 
> Do we want to comment this requirement between i915_gem_resume and
> i915_restore_state? Chris?

I'd forgotten about restore_state. That's old school save-the-world,
restore-the-world for UMS code. Keep whittling it down and eventually it
will go away, everything we do now should be independent of
i915_suspend.c...
 
> Anyway,
> 
> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Added to my queue for pushing, thanks for the patch and review.

> Regards, Joonas
> 
> PS. My guess seems to be correct according to git blame (skipping one
> rename in between) info.

Yes, debugging fences across resume has always been a case of fixing
display corruption. :|
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx