drm/i915: Rework global state serializaiton

Instead of injecting extra crtc commits to serialize the global
state let's hand roll a bit of commit machinery to take care of
the hardware synchronization.

Rather than basing everything on the crtc commits we track these
as their own thing. I think this makes more sense as the hardware
blocks we are working with are not in any way tied to the pipes,
so the completion should not be tied in with the vblank machinery
either.

The difference to the old behaviour is that:
- we no longer pull extra crtcs into the commit which should
  make drm_atomic_check_only() happier
- since those crtcs don't get pulled in we also don't end up
  reprogamming them and thus don't need to wait their vblanks
  to pass/etc. So this should be tad faster as well.

TODO: perhaps have each global object complete its own commit
once the post-plane update phase is done?

Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/6728
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20231219130756.25986-3-ville.syrjala@linux.intel.com
Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index a92e959c8ac7b4891125875fdcfbbbe5949e31cb..bc339a3589036e18e2b69ef5626e59fa803c92c8 100644 (file)
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -7077,6 +7077,7 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
 
        drm_atomic_helper_wait_for_dependencies(&state->base);
        drm_dp_mst_atomic_wait_for_dependencies(&state->base);
+       intel_atomic_global_state_wait_for_dependencies(state);
 
        /*
         * During full modesets we write a lot of registers, wait
@@ -7253,6 +7254,7 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
        intel_pmdemand_post_plane_update(state);
 
        drm_atomic_helper_commit_hw_done(&state->base);
+       intel_atomic_global_state_commit_done(state);
 
        if (state->modeset) {
                /* As one of the primary mmio accessors, KMS has a high
@@ -7303,6 +7305,21 @@ static void intel_atomic_track_fbs(struct intel_atomic_state *state)
                                        plane->frontbuffer_bit);
 }
 
+static int intel_atomic_setup_commit(struct intel_atomic_state *state, bool nonblock)
+{
+       int ret;
+
+       ret = drm_atomic_helper_setup_commit(&state->base, nonblock);
+       if (ret)
+               return ret;
+
+       ret = intel_atomic_global_state_setup_commit(state);
+       if (ret)
+               return ret;
+
+       return 0;
+}
+
 int intel_atomic_commit(struct drm_device *dev, struct drm_atomic_state *_state,
                        bool nonblock)
 {
@@ -7348,7 +7365,7 @@ int intel_atomic_commit(struct drm_device *dev, struct drm_atomic_state *_state,
                return ret;
        }
 
-       ret = drm_atomic_helper_setup_commit(&state->base, nonblock);
+       ret = intel_atomic_setup_commit(state, nonblock);
        if (!ret)
                ret = drm_atomic_helper_swap_state(&state->base, true);
        if (!ret)

diff --git a/drivers/gpu/drm/i915/display/intel_global_state.c b/drivers/gpu/drm/i915/display/intel_global_state.c
index e8e8be54143b431393bfedcac3b399429defd3ce..cbcd1e91b7bee2877fff15ff6562d20d1e7e8b7f 100644 (file)
--- a/drivers/gpu/drm/i915/display/intel_global_state.c
+++ b/drivers/gpu/drm/i915/display/intel_global_state.c
@@ -10,12 +10,55 @@
 #include "intel_display_types.h"
 #include "intel_global_state.h"
 
+struct intel_global_commit {
+       struct kref ref;
+       struct completion done;
+};
+
+static struct intel_global_commit *commit_new(void)
+{
+       struct intel_global_commit *commit;
+
+       commit = kzalloc(sizeof(*commit), GFP_KERNEL);
+       if (!commit)
+               return NULL;
+
+       init_completion(&commit->done);
+       kref_init(&commit->ref);
+
+       return commit;
+}
+
+static void __commit_free(struct kref *kref)
+{
+       struct intel_global_commit *commit =
+               container_of(kref, typeof(*commit), ref);
+
+       kfree(commit);
+}
+
+static struct intel_global_commit *commit_get(struct intel_global_commit *commit)
+{
+       if (commit)
+               kref_get(&commit->ref);
+
+       return commit;
+}
+
+static void commit_put(struct intel_global_commit *commit)
+{
+       if (commit)
+               kref_put(&commit->ref, __commit_free);
+}
+
 static void __intel_atomic_global_state_free(struct kref *kref)
 {
        struct intel_global_state *obj_state =
                container_of(kref, struct intel_global_state, ref);
        struct intel_global_obj *obj = obj_state->obj;
 
+       commit_put(obj_state->commit);
+
        obj->funcs->atomic_destroy_state(obj, obj_state);
 }
 
@@ -127,6 +170,8 @@ intel_atomic_get_global_obj_state(struct intel_atomic_state *state,
 
        obj_state->obj = obj;
        obj_state->changed = false;
+       obj_state->serialized = false;
+       obj_state->commit = NULL;
 
        kref_init(&obj_state->ref);
 
@@ -239,19 +284,13 @@ int intel_atomic_lock_global_state(struct intel_global_state *obj_state)
 
 int intel_atomic_serialize_global_state(struct intel_global_state *obj_state)
 {
-       struct intel_atomic_state *state = obj_state->state;
-       struct drm_i915_private *dev_priv = to_i915(state->base.dev);
-       struct intel_crtc *crtc;
+       int ret;
 
-       for_each_intel_crtc(&dev_priv->drm, crtc) {
-               struct intel_crtc_state *crtc_state;
+       ret = intel_atomic_lock_global_state(obj_state);
+       if (ret)
+               return ret;
 
-               crtc_state = intel_atomic_get_crtc_state(&state->base, crtc);
-               if (IS_ERR(crtc_state))
-                       return PTR_ERR(crtc_state);
-       }
-
-       obj_state->changed = true;
+       obj_state->serialized = true;
 
        return 0;
 }
@@ -267,3 +306,79 @@ intel_atomic_global_state_is_serialized(struct intel_atomic_state *state)
                        return false;
        return true;
 }
+
+int
+intel_atomic_global_state_setup_commit(struct intel_atomic_state *state)
+{
+       const struct intel_global_state *old_obj_state;
+       struct intel_global_state *new_obj_state;
+       struct intel_global_obj *obj;
+       int i;
+
+       for_each_oldnew_global_obj_in_state(state, obj, old_obj_state,
+                                           new_obj_state, i) {
+               struct intel_global_commit *commit = NULL;
+
+               if (new_obj_state->serialized) {
+                       /*
+                        * New commit which is going to be completed
+                        * after the hardware reprogramming is done.
+                        */
+                       commit = commit_new();
+                       if (!commit)
+                               return -ENOMEM;
+               } else if (new_obj_state->changed) {
+                       /*
+                        * We're going to swap to this state, so carry the
+                        * previous commit along, in case it's not yet done.
+                        */
+                       commit = commit_get(old_obj_state->commit);
+               }
+
+               new_obj_state->commit = commit;
+       }
+
+       return 0;
+}
+
+int
+intel_atomic_global_state_wait_for_dependencies(struct intel_atomic_state *state)
+{
+       struct drm_i915_private *i915 = to_i915(state->base.dev);
+       const struct intel_global_state *old_obj_state;
+       struct intel_global_obj *obj;
+       int i;
+
+       for_each_old_global_obj_in_state(state, obj, old_obj_state, i) {
+               struct intel_global_commit *commit = old_obj_state->commit;
+               long ret;
+
+               if (!commit)
+                       continue;
+
+               ret = wait_for_completion_timeout(&commit->done, 10 * HZ);
+               if (ret == 0) {
+                       drm_err(&i915->drm, "global state timed out\n");
+                       return -ETIMEDOUT;
+               }
+       }
+
+       return 0;
+}
+
+void
+intel_atomic_global_state_commit_done(struct intel_atomic_state *state)
+{
+       const struct intel_global_state *new_obj_state;
+       struct intel_global_obj *obj;
+       int i;
+
+       for_each_new_global_obj_in_state(state, obj, new_obj_state, i) {
+               struct intel_global_commit *commit = new_obj_state->commit;
+
+               if (!new_obj_state->serialized)
+                       continue;
+
+               complete_all(&commit->done);
+       }
+}

diff --git a/drivers/gpu/drm/i915/display/intel_global_state.h b/drivers/gpu/drm/i915/display/intel_global_state.h
index 5477de8f0b30ce4ec0edfc9337814365810cd5eb..5c8545d7a76a7736a7e3e782187c75adfe606d39 100644 (file)
--- a/drivers/gpu/drm/i915/display/intel_global_state.h
+++ b/drivers/gpu/drm/i915/display/intel_global_state.h
@@ -54,11 +54,14 @@ struct intel_global_obj {
             (__i)++) \
                for_each_if(obj)
 
+struct intel_global_commit;
+
 struct intel_global_state {
        struct intel_global_obj *obj;
        struct intel_atomic_state *state;
+       struct intel_global_commit *commit;
        struct kref ref;
-       bool changed;
+       bool changed, serialized;
 };
 
 struct __intel_global_objs_state {
@@ -87,6 +90,10 @@ void intel_atomic_clear_global_state(struct intel_atomic_state *state);
 int intel_atomic_lock_global_state(struct intel_global_state *obj_state);
 int intel_atomic_serialize_global_state(struct intel_global_state *obj_state);
 
+int intel_atomic_global_state_setup_commit(struct intel_atomic_state *state);
+void intel_atomic_global_state_commit_done(struct intel_atomic_state *state);
+int intel_atomic_global_state_wait_for_dependencies(struct intel_atomic_state *state);
+
 bool intel_atomic_global_state_is_serialized(struct intel_atomic_state *state);
 
 #endif