From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
With bigjoiner the master crtc is the one that will send out the
uapi event/etc. We want that to happen after all the slaves are
done, so let's try to do the commits in reverse order so that
the master comes last.
Even worse, the modeset helper will simply complete the commit
on the slave pipe immediately as it consider the crtc to be inactive
(it can't see our crtc_state->hw.active/etc.).
With regular sync updates this generally doesn't matter all that
much as the slave pipe should typically finish its work during the
same frame as the master pipe. However in case the slave pipe's commit
slips into the next frame we end up in a bit of trouble. This is most
visible with either async flips (currently disabled with bigjoiner
exactly for this reason), and DSB gamma updates. With DSB the problem
happens because the DSB itself will wait until the next start vblank
before starting to execute. So if the master pipe already finished its
commit and the DSB on the slave pipe is still waiting for the next
vblank we will assume the DSB as gotten stuck and terminate it.
Reversing the commit order should ameliarate this for the most part
as the master pipe is guaranteed to start its commit after the slave
pipe started. The one thing that can still screw us over is the fact
that we aren't necessarily going to commit the pipes in the reverse
order as the actual order is dictated by the DDB overlap avoidance.
But that can only happen while other pipes are being enabled/disabled,
and so in the normal steady state we should be safe.
The full fix will involve making the commit machinery aware of the
slave pipes and not finish their commits prematurely. But that
will involve a bit more work than this. And this commit order
reversal will still be beneficial to avoid userspace getting an
-EBUSY from the following page flip if the second pipe's commit
does stretch into the next frame.
Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
---
drivers/gpu/drm/i915/display/intel_display.c | 14 +++++++++++---
drivers/gpu/drm/i915/display/intel_display.h | 8 ++++++++
2 files changed, 19 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index a481c9218138..0086a7422e86 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -6956,8 +6956,12 @@ static void skl_commit_modeset_enables(struct intel_atomic_state *state)
intel_dbuf_mbus_pre_ddb_update(state);
while (update_pipes) {
- for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
- new_crtc_state, i) {
+ /*
+ * Commit in reverse order to make bigjoiner master
+ * send the uapi events after slaves are done.
+ */
+ for_each_oldnew_intel_crtc_in_state_reverse(state, crtc, old_crtc_state,
+ new_crtc_state, i) {
enum pipe pipe = crtc->pipe;
if ((update_pipes & BIT(pipe)) == 0)
@@ -7036,7 +7040,11 @@ static void skl_commit_modeset_enables(struct intel_atomic_state *state)
intel_pre_update_crtc(state, crtc);
}
- for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
+ /*
+ * Commit in reverse order to make bigjoiner master
+ * send the uapi events after slaves are done.
+ */
+ for_each_new_intel_crtc_in_state_reverse(state, crtc, new_crtc_state, i) {
enum pipe pipe = crtc->pipe;
if ((update_pipes & BIT(pipe)) == 0)
diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
index 986ec77490de..423074d6947a 100644
--- a/drivers/gpu/drm/i915/display/intel_display.h
+++ b/drivers/gpu/drm/i915/display/intel_display.h
@@ -344,6 +344,14 @@ enum phy_fia {
(__i)++) \
for_each_if(crtc)
+#define for_each_new_intel_crtc_in_state_reverse(__state, crtc, new_crtc_state, __i) \
+ for ((__i) = (__state)->base.dev->mode_config.num_crtc - 1; \
+ (__i) >= 0 && \
+ ((crtc) = to_intel_crtc((__state)->base.crtcs[__i].ptr), \
+ (new_crtc_state) = to_intel_crtc_state((__state)->base.crtcs[__i].new_state), 1); \
+ (__i)--) \
+ for_each_if(crtc)
+
#define for_each_oldnew_intel_plane_in_state(__state, plane, old_plane_state, new_plane_state, __i) \
for ((__i) = 0; \
(__i) < (__state)->base.dev->mode_config.num_total_plane && \
--
2.43.2
