Re: [Intel-gfx] [PATCH] drm/i915: Make lrc_init_wa_ctx compatible with ww locking, v3.

2021-02-01 Thread Chris Wilson 
Quoting Maarten Lankhorst (2021-02-01 12:50:37)
> Make creation separate from pinning, in order to take the lock only
> once, and pin the mapping with the lock held.
> 
> Changes since v1:
> - Rebase on top of upstream changes.
> Changes since v2:
> - Fully clear wa_ctx on error.
> 
> Signed-off-by: Maarten Lankhorst 
> Reviewed-by: Thomas Hellström 
> ---
>  drivers/gpu/drm/i915/gt/intel_lrc.c | 49 ++---
>  1 file changed, 38 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c 
> b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index 8508b8d701c1..a2b916d27a39 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -1421,7 +1421,7 @@ gen10_init_indirectctx_bb(struct intel_engine_cs 
> *engine, u32 *batch)
>  
>  #define CTX_WA_BB_SIZE (PAGE_SIZE)
>  
> -static int lrc_setup_wa_ctx(struct intel_engine_cs *engine)
> +static int lrc_create_wa_ctx(struct intel_engine_cs *engine)
>  {
> struct drm_i915_gem_object *obj;
> struct i915_vma *vma;
> @@ -1437,10 +1437,6 @@ static int lrc_setup_wa_ctx(struct intel_engine_cs 
> *engine)
> goto err;
> }
>  
> -   err = i915_ggtt_pin(vma, NULL, 0, PIN_HIGH);
> -   if (err)
> -   goto err;
> -
> engine->wa_ctx.vma = vma;
> return 0;
>  
> @@ -1452,9 +1448,6 @@ static int lrc_setup_wa_ctx(struct intel_engine_cs 
> *engine)
>  void lrc_fini_wa_ctx(struct intel_engine_cs *engine)
>  {
> i915_vma_unpin_and_release(>wa_ctx.vma, 0);
> -
> -   /* Called on error unwind, clear all flags to prevent further use */
> -   memset(>wa_ctx, 0, sizeof(engine->wa_ctx));
>  }
>  
>  typedef u32 *(*wa_bb_func_t)(struct intel_engine_cs *engine, u32 *batch);
> @@ -1466,6 +1459,7 @@ void lrc_init_wa_ctx(struct intel_engine_cs *engine)
> _ctx->indirect_ctx, _ctx->per_ctx
> };
> wa_bb_func_t wa_bb_fn[ARRAY_SIZE(wa_bb)];
> +   struct i915_gem_ww_ctx ww;
> void *batch, *batch_ptr;
> unsigned int i;
> int err;
> @@ -1494,7 +1488,7 @@ void lrc_init_wa_ctx(struct intel_engine_cs *engine)
> return;
> }
>  
> -   err = lrc_setup_wa_ctx(engine);
> +   err = lrc_create_wa_ctx(engine);
> if (err) {
> /*
>  * We continue even if we fail to initialize WA batch
> @@ -1507,7 +1501,22 @@ void lrc_init_wa_ctx(struct intel_engine_cs *engine)
> return;
> }
>  
> +   if (!engine->wa_ctx.vma)
> +   return;
> +
> +   i915_gem_ww_ctx_init(, true);
> +retry:
> +   err = i915_gem_object_lock(wa_ctx->vma->obj, );
> +   if (!err)
> +   err = i915_ggtt_pin(wa_ctx->vma, , 0, PIN_HIGH);
> +   if (err)
> +   goto err;
> +
> batch = i915_gem_object_pin_map(wa_ctx->vma->obj, I915_MAP_WB);

Given that the pages are already pinned and must remain pinned for the
lifetime of the engine, and the ww is not used here to setup the CPU
page tables, fix the lack of primitives.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Make lrc_init_wa_ctx compatible with ww locking, v3.

2021-02-01 Thread Thomas Hellström 


On 2/1/21 1:50 PM, Maarten Lankhorst wrote:

Make creation separate from pinning, in order to take the lock only
once, and pin the mapping with the lock held.

Changes since v1:
- Rebase on top of upstream changes.
Changes since v2:
- Fully clear wa_ctx on error.

Signed-off-by: Maarten Lankhorst 
Reviewed-by: Thomas Hellström 
---


LGTM.

Thomas


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

[Intel-gfx] [PATCH] drm/i915: Make lrc_init_wa_ctx compatible with ww locking, v3.

2021-02-01 Thread Maarten Lankhorst 
Make creation separate from pinning, in order to take the lock only
once, and pin the mapping with the lock held.

Changes since v1:
- Rebase on top of upstream changes.
Changes since v2:
- Fully clear wa_ctx on error.

Signed-off-by: Maarten Lankhorst 
Reviewed-by: Thomas Hellström 
---
 drivers/gpu/drm/i915/gt/intel_lrc.c | 49 ++---
 1 file changed, 38 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c 
b/drivers/gpu/drm/i915/gt/intel_lrc.c
index 8508b8d701c1..a2b916d27a39 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -1421,7 +1421,7 @@ gen10_init_indirectctx_bb(struct intel_engine_cs *engine, 
u32 *batch)
 
 #define CTX_WA_BB_SIZE (PAGE_SIZE)
 
-static int lrc_setup_wa_ctx(struct intel_engine_cs *engine)
+static int lrc_create_wa_ctx(struct intel_engine_cs *engine)
 {
struct drm_i915_gem_object *obj;
struct i915_vma *vma;
@@ -1437,10 +1437,6 @@ static int lrc_setup_wa_ctx(struct intel_engine_cs 
*engine)
goto err;
}
 
-   err = i915_ggtt_pin(vma, NULL, 0, PIN_HIGH);
-   if (err)
-   goto err;
-
engine->wa_ctx.vma = vma;
return 0;
 
@@ -1452,9 +1448,6 @@ static int lrc_setup_wa_ctx(struct intel_engine_cs 
*engine)
 void lrc_fini_wa_ctx(struct intel_engine_cs *engine)
 {
i915_vma_unpin_and_release(>wa_ctx.vma, 0);
-
-   /* Called on error unwind, clear all flags to prevent further use */
-   memset(>wa_ctx, 0, sizeof(engine->wa_ctx));
 }
 
 typedef u32 *(*wa_bb_func_t)(struct intel_engine_cs *engine, u32 *batch);
@@ -1466,6 +1459,7 @@ void lrc_init_wa_ctx(struct intel_engine_cs *engine)
_ctx->indirect_ctx, _ctx->per_ctx
};
wa_bb_func_t wa_bb_fn[ARRAY_SIZE(wa_bb)];
+   struct i915_gem_ww_ctx ww;
void *batch, *batch_ptr;
unsigned int i;
int err;
@@ -1494,7 +1488,7 @@ void lrc_init_wa_ctx(struct intel_engine_cs *engine)
return;
}
 
-   err = lrc_setup_wa_ctx(engine);
+   err = lrc_create_wa_ctx(engine);
if (err) {
/*
 * We continue even if we fail to initialize WA batch
@@ -1507,7 +1501,22 @@ void lrc_init_wa_ctx(struct intel_engine_cs *engine)
return;
}
 
+   if (!engine->wa_ctx.vma)
+   return;
+
+   i915_gem_ww_ctx_init(, true);
+retry:
+   err = i915_gem_object_lock(wa_ctx->vma->obj, );
+   if (!err)
+   err = i915_ggtt_pin(wa_ctx->vma, , 0, PIN_HIGH);
+   if (err)
+   goto err;
+
batch = i915_gem_object_pin_map(wa_ctx->vma->obj, I915_MAP_WB);
+   if (IS_ERR(batch)) {
+   err = PTR_ERR(batch);
+   goto err_unpin;
+   }
 
/*
 * Emit the two workaround batch buffers, recording the offset from the
@@ -1532,8 +1541,26 @@ void lrc_init_wa_ctx(struct intel_engine_cs *engine)
__i915_gem_object_release_map(wa_ctx->vma->obj);
 
/* Verify that we can handle failure to setup the wa_ctx */
-   if (err || i915_inject_probe_error(engine->i915, -ENODEV))
-   lrc_fini_wa_ctx(engine);
+   if (!err)
+   err = i915_inject_probe_error(engine->i915, -ENODEV);
+
+err_unpin:
+   if (err)
+   i915_vma_unpin(wa_ctx->vma);
+err:
+   if (err == -EDEADLK) {
+   err = i915_gem_ww_ctx_backoff();
+   if (!err)
+   goto retry;
+   }
+   i915_gem_ww_ctx_fini();
+
+   if (err) {
+   i915_vma_put(engine->wa_ctx.vma);
+
+   /* Clear all flags to prevent further use */
+   memset(wa_ctx, 0, sizeof(*wa_ctx));
+   }
 }
 
 static void st_runtime_underflow(struct intel_context_stats *stats, s32 dt)
-- 
2.30.0

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