[PATCH 1/3] drm/i915: Repeat flush of idle work during suspend

[PATCH 1/3] drm/i915: Repeat flush of idle work during suspend

[Chris Wilson]

The idle work handler is self-arming - if it detects that it needs to
run again it will queue itself from its work handler. Take greater care
when trying to drain the idle work, and double check that it is flushed.

The free worker has a similar issue where it is armed by an RCU task
which may be running concurrently with us.

This should hopefully help with the sporadic WARN_ON(dev_priv->gt.awake)
from i915_gem_suspend.

v2: Reuse drain_freed_objects.
v3: Don't try to flush the freed objects from the shrinker, as it may be
underneath the struct_mutex already.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.c | 3 +--
 drivers/gpu/drm/i915/i915_drv.h | 7 +++++++
 drivers/gpu/drm/i915/i915_gem.c | 6 ++++--
 3 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 6428588518aa..2c020eafada6 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -545,8 +545,7 @@ static void i915_gem_fini(struct drm_i915_private *dev_priv)
 	i915_gem_context_fini(dev_priv);
 	mutex_unlock(&dev_priv->drm.struct_mutex);
 
-	rcu_barrier();
-	flush_work(&dev_priv->mm.free_work);
+	i915_gem_drain_freed_objects(dev_priv);
 
 	WARN_ON(!list_empty(&dev_priv->context_list));
 }
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 77d7a079c51b..37ea557d3cd4 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3207,6 +3207,13 @@ i915_gem_object_create_from_data(struct drm_i915_private *dev_priv,
 void i915_gem_close_object(struct drm_gem_object *gem, struct drm_file *file);
 void i915_gem_free_object(struct drm_gem_object *obj);
 
+static inline void i915_gem_drain_freed_objects(struct drm_i915_private *i915)
+{
+	rcu_barrier();
+	while (flush_work(&i915->mm.free_work))
+		rcu_barrier();
+}
+
 struct i915_vma * __must_check
 i915_gem_object_ggtt_pin(struct drm_i915_gem_object *obj,
 			 const struct i915_ggtt_view *view,
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index cc4e0224968f..62e1af5dd1b0 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4262,8 +4262,10 @@ int i915_gem_suspend(struct drm_i915_private *dev_priv)
 
 	cancel_delayed_work_sync(&dev_priv->gpu_error.hangcheck_work);
 	cancel_delayed_work_sync(&dev_priv->gt.retire_work);
-	flush_delayed_work(&dev_priv->gt.idle_work);
-	flush_work(&dev_priv->mm.free_work);
+	while (flush_delayed_work(&dev_priv->gt.idle_work))
+		;
+
+	i915_gem_drain_freed_objects(dev_priv);
 
 	/* Assert that we sucessfully flushed all the work and
 	 * reset the GPU back to its idle, low power state.
-- 
2.11.0

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

[PATCH 2/3] drm/i915: Don't clflush before release phys object

[Chris Wilson]

When we teardown the backing storage for the phys object, we copy from
the coherent contiguous block back to the shmemfs object, clflushing as
we go. Trying to clflush the invalid sg beforehand just oops and would
be redundant (due to it already being coherent, and clflushed
afterwards).

Reported-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: <drm-intel-fixes@lists.freedesktop.org>
---
 drivers/gpu/drm/i915/i915_gem.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 62e1af5dd1b0..910cbeef2959 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -244,14 +244,16 @@ i915_gem_object_get_pages_phys(struct drm_i915_gem_object *obj)
 
 static void
 __i915_gem_object_release_shmem(struct drm_i915_gem_object *obj,
-				struct sg_table *pages)
+				struct sg_table *pages,
+				bool needs_clflush)
 {
 	GEM_BUG_ON(obj->mm.madv == __I915_MADV_PURGED);
 
 	if (obj->mm.madv == I915_MADV_DONTNEED)
 		obj->mm.dirty = false;
 
-	if ((obj->base.read_domains & I915_GEM_DOMAIN_CPU) == 0 &&
+	if (needs_clflush &&
+	    (obj->base.read_domains & I915_GEM_DOMAIN_CPU) == 0 &&
 	    !cpu_cache_is_coherent(obj->base.dev, obj->cache_level))
 		drm_clflush_sg(pages);
 
@@ -263,7 +265,7 @@ static void
 i915_gem_object_put_pages_phys(struct drm_i915_gem_object *obj,
 			       struct sg_table *pages)
 {
-	__i915_gem_object_release_shmem(obj, pages);
+	__i915_gem_object_release_shmem(obj, pages, false);
 
 	if (obj->mm.dirty) {
 		struct address_space *mapping = obj->base.filp->f_mapping;
@@ -2231,7 +2233,7 @@ i915_gem_object_put_pages_gtt(struct drm_i915_gem_object *obj,
 	struct sgt_iter sgt_iter;
 	struct page *page;
 
-	__i915_gem_object_release_shmem(obj, pages);
+	__i915_gem_object_release_shmem(obj, pages, true);
 
 	i915_gem_gtt_finish_pages(obj, pages);
 
-- 
2.11.0

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

[PATCH 3/3] drm/i915: Silence allocation failure during sg_trim()

[Chris Wilson]

As trimming the sg table is merely an optimisation that gracefully fails
if we cannot allocate a new table, we do not need to report the failure
either.

Fixes: 0c40ce130e38 ("drm/i915: Trim the object sg table")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 910cbeef2959..32da5ac29cdb 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2324,7 +2324,7 @@ static void i915_sg_trim(struct sg_table *orig_st)
 	if (orig_st->nents == orig_st->orig_nents)
 		return;
 
-	if (sg_alloc_table(&new_st, orig_st->nents, GFP_KERNEL))
+	if (sg_alloc_table(&new_st, orig_st->nents, GFP_KERNEL | __GFP_NOWARN))
 		return;
 
 	new_sg = new_st.sgl;
-- 
2.11.0

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

Re: [PATCH 3/3] drm/i915: Silence allocation failure during sg_trim()

[Joonas Lahtinen]

On to, 2016-12-22 at 12:00 +0000, Chris Wilson wrote:
> As trimming the sg table is merely an optimisation that gracefully fails
> if we cannot allocate a new table, we do not need to report the failure
> either.
> 
> Fixes: 0c40ce130e38 ("drm/i915: Trim the object sg table")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

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

Regards, Joonas
-- 
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/3] drm/i915: Repeat flush of idle work during suspend

[Joonas Lahtinen]

On to, 2016-12-22 at 12:00 +0000, Chris Wilson wrote:
> The idle work handler is self-arming - if it detects that it needs to
> run again it will queue itself from its work handler. Take greater care
> when trying to drain the idle work, and double check that it is flushed.
> 
> The free worker has a similar issue where it is armed by an RCU task
> which may be running concurrently with us.
> 
> This should hopefully help with the sporadic WARN_ON(dev_priv->gt.awake)
> from i915_gem_suspend.
> 
> v2: Reuse drain_freed_objects.
> v3: Don't try to flush the freed objects from the shrinker, as it may be
> underneath the struct_mutex already.

<SNIP>

> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3207,6 +3207,13 @@ i915_gem_object_create_from_data(struct drm_i915_private *dev_priv,
>  void i915_gem_close_object(struct drm_gem_object *gem, struct drm_file *file);
>  void i915_gem_free_object(struct drm_gem_object *obj);
>  
> +static inline void i915_gem_drain_freed_objects(struct drm_i915_private *i915)
> +{
> +	rcu_barrier();
> +	while (flush_work(&i915->mm.free_work))
> +		rcu_barrier();

Looks much like do { } while();

Also make a comment that we loop just for paranoia.

> @@ -4262,8 +4262,10 @@ int i915_gem_suspend(struct drm_i915_private *dev_priv)
>  
>  	cancel_delayed_work_sync(&dev_priv->gpu_error.hangcheck_work);
>  	cancel_delayed_work_sync(&dev_priv->gt.retire_work);
> -	flush_delayed_work(&dev_priv->gt.idle_work);
> -	flush_work(&dev_priv->mm.free_work);

Put a comment here that idle_work is re-arming.

> +	while (flush_delayed_work(&dev_priv->gt.idle_work))
> +		;
> +
> +	i915_gem_drain_freed_objects(dev_priv);
>  
>  	/* Assert that we sucessfully flushed all the work and
>  	 * reset the GPU back to its idle, low power state.

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

Regards, Joonas
-- 
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 2/3] drm/i915: Don't clflush before release phys object

[Joonas Lahtinen]

On to, 2016-12-22 at 12:00 +0000, Chris Wilson wrote:
> When we teardown the backing storage for the phys object, we copy from
> the coherent contiguous block back to the shmemfs object, clflushing as
> we go. Trying to clflush the invalid sg beforehand just oops and would
> be redundant (due to it already being coherent, and clflushed
> afterwards).
> 
> Reported-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: <drm-intel-fixes@lists.freedesktop.org>

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

Regards, Joonas
-- 
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

✗ Fi.CI.BAT: warning for series starting with [1/3] drm/i915: Repeat flush of idle work during suspend

[Patchwork]

== Series Details ==

Series: series starting with [1/3] drm/i915: Repeat flush of idle work during suspend
URL   : https://patchwork.freedesktop.org/series/17131/
State : warning

== Summary ==

Series 17131v1 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/17131/revisions/1/mbox/

Test kms_pipe_crc_basic:
        Subgroup nonblocking-crc-pipe-b-frame-sequence:
                pass       -> DMESG-WARN (fi-snb-2520m)

fi-bdw-5557u     total:246  pass:232  dwarn:0   dfail:0   fail:0   skip:14 
fi-bsw-n3050     total:246  pass:207  dwarn:0   dfail:0   fail:0   skip:39 
fi-bxt-j4205     total:246  pass:224  dwarn:0   dfail:0   fail:0   skip:22 
fi-bxt-t5700     total:246  pass:219  dwarn:0   dfail:0   fail:0   skip:27 
fi-byt-j1900     total:246  pass:219  dwarn:0   dfail:0   fail:0   skip:27 
fi-byt-n2820     total:246  pass:215  dwarn:0   dfail:0   fail:0   skip:31 
fi-hsw-4770      total:246  pass:227  dwarn:0   dfail:0   fail:0   skip:19 
fi-hsw-4770r     total:246  pass:227  dwarn:0   dfail:0   fail:0   skip:19 
fi-ilk-650       total:246  pass:194  dwarn:0   dfail:0   fail:0   skip:52 
fi-ivb-3520m     total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-ivb-3770      total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-kbl-7500u     total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-skl-6260u     total:246  pass:233  dwarn:0   dfail:0   fail:0   skip:13 
fi-skl-6700hq    total:246  pass:226  dwarn:0   dfail:0   fail:0   skip:20 
fi-skl-6700k     total:246  pass:222  dwarn:3   dfail:0   fail:0   skip:21 
fi-skl-6770hq    total:246  pass:233  dwarn:0   dfail:0   fail:0   skip:13 
fi-snb-2520m     total:246  pass:214  dwarn:1   dfail:0   fail:0   skip:31 
fi-snb-2600      total:246  pass:214  dwarn:0   dfail:0   fail:0   skip:32 

5486838e15ec59a57aa9a62f049f383cc3968e3f drm-tip: 2016y-12m-22d-13h-47m-26s UTC integration manifest
5a4522e drm/i915: Silence allocation failure during sg_trim()
21e42f9 drm/i915: Don't clflush before release phys object
bcb41cb drm/i915: Repeat flush of idle work during suspend

== Logs ==

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