On 08/04/2019 10:17, Chris Wilson wrote:
Move the intel_context_instance() to the caller so that we can decouple
ourselves from one context instance per engine.

Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
---
  drivers/gpu/drm/i915/gt/intel_context.c       | 26 ------
  drivers/gpu/drm/i915/gt/intel_context.h       | 14 ++-
  drivers/gpu/drm/i915/i915_gem_context.c       | 88 +++++++++++--------
  .../gpu/drm/i915/selftests/i915_gem_context.c |  2 +-
  4 files changed, 64 insertions(+), 66 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_context.c 
b/drivers/gpu/drm/i915/gt/intel_context.c
index a1267739e369..46bf8d8fb7e4 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.c
+++ b/drivers/gpu/drm/i915/gt/intel_context.c
@@ -128,32 +128,6 @@ intel_context_instance(struct i915_gem_context *ctx,
        return intel_context_get(pos);
  }
-struct intel_context *
-intel_context_pin_lock(struct i915_gem_context *ctx,
-                      struct intel_engine_cs *engine)
-       __acquires(ce->pin_mutex)
-{
-       struct intel_context *ce;
-
-       ce = intel_context_instance(ctx, engine);
-       if (IS_ERR(ce))
-               return ce;
-
-       if (mutex_lock_interruptible(&ce->pin_mutex)) {
-               intel_context_put(ce);
-               return ERR_PTR(-EINTR);
-       }
-
-       return ce;
-}
-
-void intel_context_pin_unlock(struct intel_context *ce)
-       __releases(ce->pin_mutex)
-{
-       mutex_unlock(&ce->pin_mutex);
-       intel_context_put(ce);
-}
-
  int __intel_context_do_pin(struct intel_context *ce)
  {
        int err;
diff --git a/drivers/gpu/drm/i915/gt/intel_context.h 
b/drivers/gpu/drm/i915/gt/intel_context.h
index da342e9a8c2e..4f8ef45e6344 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.h
+++ b/drivers/gpu/drm/i915/gt/intel_context.h
@@ -39,9 +39,17 @@ intel_context_lookup(struct i915_gem_context *ctx,
   * can neither be bound to the GPU or unbound whilst the lock is held, i.e.
   * intel_context_is_pinned() remains stable.
   */
-struct intel_context *
-intel_context_pin_lock(struct i915_gem_context *ctx,
-                      struct intel_engine_cs *engine);
+static inline int intel_context_pin_lock(struct intel_context *ce)
+       __acquires(ce->pin_mutex)
+{
+       return mutex_lock_interruptible(&ce->pin_mutex);
+}

So if this is not getting the kref any more why are the other callers okay? In previous patch(es) some certainly looked like they assume pin implies a reference.

+
+static inline void intel_context_pin_unlock(struct intel_context *ce)
+       __releases(ce->pin_mutex)
+{
+       mutex_unlock(&ce->pin_mutex);
+}
static inline bool
  intel_context_is_pinned(struct intel_context *ce)
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
b/drivers/gpu/drm/i915/i915_gem_context.c
index 51c427ac0c21..1e84fe0a4c55 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -141,6 +141,18 @@ static void lut_close(struct i915_gem_context *ctx)
        rcu_read_unlock();
  }
+static struct intel_context *
+lookup_user_engine(struct i915_gem_context *ctx, u16 class, u16 instance)
+{
+       struct intel_engine_cs *engine;
+
+       engine = intel_engine_lookup_user(ctx->i915, class, instance);
+       if (!engine)
+               return ERR_PTR(-EINVAL);
+
+       return intel_context_instance(ctx, engine);
+}
+
  static inline int new_hw_id(struct drm_i915_private *i915, gfp_t gfp)
  {
        unsigned int max;
@@ -1142,19 +1154,17 @@ gen8_modify_rpcs(struct intel_context *ce, struct 
intel_sseu sseu)
  }
static int
-__i915_gem_context_reconfigure_sseu(struct i915_gem_context *ctx,
-                                   struct intel_engine_cs *engine,
-                                   struct intel_sseu sseu)
+__intel_context_reconfigure_sseu(struct intel_context *ce,
+                                struct intel_sseu sseu)
  {
-       struct intel_context *ce;
-       int ret = 0;
+       int ret;
- GEM_BUG_ON(INTEL_GEN(ctx->i915) < 8);
-       GEM_BUG_ON(engine->id != RCS0);
+       GEM_BUG_ON(INTEL_GEN(ce->gem_context->i915) < 8);
+       GEM_BUG_ON(ce->engine->id != RCS0);
- ce = intel_context_pin_lock(ctx, engine);
-       if (IS_ERR(ce))
-               return PTR_ERR(ce);
+       ret = intel_context_pin_lock(ce);
+       if (ret)
+               return ret;
/* Nothing to do if unmodified. */
        if (!memcmp(&ce->sseu, &sseu, sizeof(sseu)))
@@ -1170,19 +1180,18 @@ __i915_gem_context_reconfigure_sseu(struct 
i915_gem_context *ctx,
  }
static int
-i915_gem_context_reconfigure_sseu(struct i915_gem_context *ctx,
-                                 struct intel_engine_cs *engine,
-                                 struct intel_sseu sseu)
+intel_context_reconfigure_sseu(struct intel_context *ce, struct intel_sseu 
sseu)
  {
+       struct drm_i915_private *i915 = ce->gem_context->i915;
        int ret;
- ret = mutex_lock_interruptible(&ctx->i915->drm.struct_mutex);
+       ret = mutex_lock_interruptible(&i915->drm.struct_mutex);
        if (ret)
                return ret;
- ret = __i915_gem_context_reconfigure_sseu(ctx, engine, sseu);
+       ret = __intel_context_reconfigure_sseu(ce, sseu);
- mutex_unlock(&ctx->i915->drm.struct_mutex);
+       mutex_unlock(&i915->drm.struct_mutex);
return ret;
  }
@@ -1290,7 +1299,7 @@ static int set_sseu(struct i915_gem_context *ctx,
  {
        struct drm_i915_private *i915 = ctx->i915;
        struct drm_i915_gem_context_param_sseu user_sseu;
-       struct intel_engine_cs *engine;
+       struct intel_context *ce;
        struct intel_sseu sseu;
        int ret;
@@ -1307,27 +1316,31 @@ static int set_sseu(struct i915_gem_context *ctx,
        if (user_sseu.flags || user_sseu.rsvd)
                return -EINVAL;
- engine = intel_engine_lookup_user(i915,
-                                         user_sseu.engine_class,
-                                         user_sseu.engine_instance);
-       if (!engine)
-               return -EINVAL;
+       ce = lookup_user_engine(ctx,
+                               user_sseu.engine_class,
+                               user_sseu.engine_instance);
+       if (IS_ERR(ce))
+               return PTR_ERR(ce);
/* Only render engine supports RPCS configuration. */
-       if (engine->class != RENDER_CLASS)
-               return -ENODEV;
+       if (ce->engine->class != RENDER_CLASS) {
+               ret = -ENODEV;
+               goto out_ce;
+       }
ret = user_to_context_sseu(i915, &user_sseu, &sseu);
        if (ret)
-               return ret;
+               goto out_ce;
- ret = i915_gem_context_reconfigure_sseu(ctx, engine, sseu);
+       ret = intel_context_reconfigure_sseu(ce, sseu);
        if (ret)
-               return ret;
+               goto out_ce;
args->size = sizeof(user_sseu); - return 0;
+out_ce:
+       intel_context_put(ce);
+       return ret;
  }
static int ctx_setparam(struct drm_i915_file_private *fpriv,
@@ -1532,8 +1545,8 @@ static int get_sseu(struct i915_gem_context *ctx,
                    struct drm_i915_gem_context_param *args)
  {
        struct drm_i915_gem_context_param_sseu user_sseu;
-       struct intel_engine_cs *engine;
        struct intel_context *ce;
+       int err;
if (args->size == 0)
                goto out;
@@ -1547,22 +1560,25 @@ static int get_sseu(struct i915_gem_context *ctx,
        if (user_sseu.flags || user_sseu.rsvd)
                return -EINVAL;
- engine = intel_engine_lookup_user(ctx->i915,
-                                         user_sseu.engine_class,
-                                         user_sseu.engine_instance);
-       if (!engine)
-               return -EINVAL;
-
-       ce = intel_context_pin_lock(ctx, engine); /* serialises with set_sseu */
+       ce = lookup_user_engine(ctx,
+                               user_sseu.engine_class,
+                               user_sseu.engine_instance);
        if (IS_ERR(ce))
                return PTR_ERR(ce);
+ err = intel_context_pin_lock(ce); /* serialises with set_sseu */
+       if (err) {
+               intel_context_put(ce);
+               return err;
+       }
+
        user_sseu.slice_mask = ce->sseu.slice_mask;
        user_sseu.subslice_mask = ce->sseu.subslice_mask;
        user_sseu.min_eus_per_subslice = ce->sseu.min_eus_per_subslice;
        user_sseu.max_eus_per_subslice = ce->sseu.max_eus_per_subslice;
intel_context_pin_unlock(ce);
+       intel_context_put(ce);
if (copy_to_user(u64_to_user_ptr(args->value), &user_sseu,
                         sizeof(user_sseu)))
diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_context.c 
b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
index 8e2a94333559..214d1fd2f4dc 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
@@ -1017,7 +1017,7 @@ __sseu_test(struct drm_i915_private *i915,
        if (ret)
                return ret;
- ret = __i915_gem_context_reconfigure_sseu(ce->gem_context, ce->engine, sseu);
+       ret = __intel_context_reconfigure_sseu(ce, sseu);
        if (ret)
                goto out_spin;

Rest looks good.

Regards,

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

Reply via email to