Re: [Intel-gfx] [PATCH] drm/i915: Pull unpin map into vma release

2018-07-24 Thread Chris Wilson 
Quoting Michał Winiarski (2018-07-24 09:48:59)
> On Sat, Jul 21, 2018 at 01:50:37PM +0100, Chris Wilson wrote:
> > A reasonably common operation is to pin the map of the vma alongside the
> > vma itself for the lifetime of the vma, and so release both pin at the
> > same time as destroying the vma. It is common enough to pull into the
> > release function, making that central function more attractive to a
> > couple of other callsites.
> > 
> > The continual ulterior motive is to sweep over errors on module load
> > aborting...
> > 
> > Testcase: igt/drv_module_reload/basic-reload-inject
> > Signed-off-by: Chris Wilson 
> > Cc: Michał Winiarski 
> > Cc: Michal Wajdeczko 
> 
> Reviewed-by: Michał Winiarski 

Hopefully this was the last oops for drv_module_reload...
Now just need a random patch for igt, take your pick of
https://patchwork.freedesktop.org/series/47084/
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Pull unpin map into vma release

2018-07-24 Thread Michał Winiarski 
On Sat, Jul 21, 2018 at 01:50:37PM +0100, Chris Wilson wrote:
> A reasonably common operation is to pin the map of the vma alongside the
> vma itself for the lifetime of the vma, and so release both pin at the
> same time as destroying the vma. It is common enough to pull into the
> release function, making that central function more attractive to a
> couple of other callsites.
> 
> The continual ulterior motive is to sweep over errors on module load
> aborting...
> 
> Testcase: igt/drv_module_reload/basic-reload-inject
> Signed-off-by: Chris Wilson 
> Cc: Michał Winiarski 
> Cc: Michal Wajdeczko 

Reviewed-by: Michał Winiarski 

-Michał

> ---
>  drivers/gpu/drm/i915/i915_perf.c| 10 --
>  drivers/gpu/drm/i915/i915_vma.c |  5 -
>  drivers/gpu/drm/i915/i915_vma.h |  3 ++-
>  drivers/gpu/drm/i915/intel_engine_cs.c  | 18 +++---
>  drivers/gpu/drm/i915/intel_guc.c|  5 ++---
>  drivers/gpu/drm/i915/intel_guc_ads.c|  2 +-
>  drivers/gpu/drm/i915/intel_guc_ct.c |  7 ++-
>  drivers/gpu/drm/i915/intel_guc_log.c|  2 +-
>  drivers/gpu/drm/i915/intel_guc_submission.c | 10 --
>  drivers/gpu/drm/i915/intel_lrc.c|  2 +-
>  10 files changed, 24 insertions(+), 40 deletions(-)
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915: Pull unpin map into vma release

2018-07-21 Thread Chris Wilson 
A reasonably common operation is to pin the map of the vma alongside the
vma itself for the lifetime of the vma, and so release both pin at the
same time as destroying the vma. It is common enough to pull into the
release function, making that central function more attractive to a
couple of other callsites.

The continual ulterior motive is to sweep over errors on module load
aborting...

Testcase: igt/drv_module_reload/basic-reload-inject
Signed-off-by: Chris Wilson 
Cc: Michał Winiarski 
Cc: Michal Wajdeczko 
---
 drivers/gpu/drm/i915/i915_perf.c| 10 --
 drivers/gpu/drm/i915/i915_vma.c |  5 -
 drivers/gpu/drm/i915/i915_vma.h |  3 ++-
 drivers/gpu/drm/i915/intel_engine_cs.c  | 18 +++---
 drivers/gpu/drm/i915/intel_guc.c|  5 ++---
 drivers/gpu/drm/i915/intel_guc_ads.c|  2 +-
 drivers/gpu/drm/i915/intel_guc_ct.c |  7 ++-
 drivers/gpu/drm/i915/intel_guc_log.c|  2 +-
 drivers/gpu/drm/i915/intel_guc_submission.c | 10 --
 drivers/gpu/drm/i915/intel_lrc.c|  2 +-
 10 files changed, 24 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 6bf10952c724..0376338d1f8d 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -1338,14 +1338,12 @@ free_oa_buffer(struct drm_i915_private *i915)
 {
mutex_lock(>drm.struct_mutex);
 
-   i915_gem_object_unpin_map(i915->perf.oa.oa_buffer.vma->obj);
-   i915_vma_unpin(i915->perf.oa.oa_buffer.vma);
-   i915_gem_object_put(i915->perf.oa.oa_buffer.vma->obj);
-
-   i915->perf.oa.oa_buffer.vma = NULL;
-   i915->perf.oa.oa_buffer.vaddr = NULL;
+   i915_vma_unpin_and_release(>perf.oa.oa_buffer.vma,
+  I915_VMA_RELEASE_MAP);
 
mutex_unlock(>drm.struct_mutex);
+
+   i915->perf.oa.oa_buffer.vaddr = NULL;
 }
 
 static void i915_oa_stream_destroy(struct i915_perf_stream *stream)
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index 11d834f94220..274fd2a7bcb6 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -406,7 +406,7 @@ void i915_vma_unpin_iomap(struct i915_vma *vma)
i915_vma_unpin(vma);
 }
 
-void i915_vma_unpin_and_release(struct i915_vma **p_vma)
+void i915_vma_unpin_and_release(struct i915_vma **p_vma, unsigned int flags)
 {
struct i915_vma *vma;
struct drm_i915_gem_object *obj;
@@ -421,6 +421,9 @@ void i915_vma_unpin_and_release(struct i915_vma **p_vma)
i915_vma_unpin(vma);
i915_vma_close(vma);
 
+   if (flags & I915_VMA_RELEASE_MAP)
+   i915_gem_object_unpin_map(obj);
+
__i915_gem_object_release_unless_active(obj);
 }
 
diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
index f06d66377107..af5296b015f5 100644
--- a/drivers/gpu/drm/i915/i915_vma.h
+++ b/drivers/gpu/drm/i915/i915_vma.h
@@ -138,7 +138,8 @@ i915_vma_instance(struct drm_i915_gem_object *obj,
  struct i915_address_space *vm,
  const struct i915_ggtt_view *view);
 
-void i915_vma_unpin_and_release(struct i915_vma **p_vma);
+void i915_vma_unpin_and_release(struct i915_vma **p_vma, unsigned int flags);
+#define I915_VMA_RELEASE_MAP BIT(0)
 
 static inline bool i915_vma_is_active(struct i915_vma *vma)
 {
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c 
b/drivers/gpu/drm/i915/intel_engine_cs.c
index 2d1952849d69..734a789688da 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -527,7 +527,7 @@ int intel_engine_create_scratch(struct intel_engine_cs 
*engine,
 
 void intel_engine_cleanup_scratch(struct intel_engine_cs *engine)
 {
-   i915_vma_unpin_and_release(>scratch);
+   i915_vma_unpin_and_release(>scratch, 0);
 }
 
 static void cleanup_phys_status_page(struct intel_engine_cs *engine)
@@ -543,20 +543,8 @@ static void cleanup_phys_status_page(struct 
intel_engine_cs *engine)
 
 static void cleanup_status_page(struct intel_engine_cs *engine)
 {
-   struct i915_vma *vma;
-   struct drm_i915_gem_object *obj;
-
-   vma = fetch_and_zero(>status_page.vma);
-   if (!vma)
-   return;
-
-   obj = vma->obj;
-
-   i915_vma_unpin(vma);
-   i915_vma_close(vma);
-
-   i915_gem_object_unpin_map(obj);
-   __i915_gem_object_release_unless_active(obj);
+   i915_vma_unpin_and_release(>status_page.vma,
+  I915_VMA_RELEASE_MAP);
 }
 
 static int init_status_page(struct intel_engine_cs *engine)
diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
index 560c7406ae40..846d693ecb53 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -170,7 +170,7 @@ static int guc_shared_data_create(struct intel_guc *guc)
 
vaddr = i915_gem_object_pin_map(vma->obj,