From 482bcc7ee600dcb5fb937dc742d051977fb5983c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Wed, 8 Apr 2026 18:57:43 +0300 Subject: [PATCH 1/3] drm/i915/joiner: Make joiner "nomodeset" state copy independent of pipe order MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently the joiner primary->secondary hw state copy still happens from the main compute_config loop alongside the primary uapi->hw state copy. The primary uapi->hw state copy must therefore happen first, or else we'll end up copying stale junk into the secondary. We have a WARN in intel_atomic_check_joiner() to make sure the CRTCs will be walked in the correct order. The plan is to reoder the CRTCs, which would mess up the order, unless we also adjust the iterators to keep the pipe order. The actual plan is to do both, so technically we should be able to just remove the WARN and call it a day. But relying on the iteration order like this is fragile and confusing, so let's move the "nomodeset" joiner state copy into the later loop where the "modeset" state copy is also done. The first loop having completely finished, we are guaranteed to have up to date hw state on the primary when we do the copy to the secondary. Cc: Jani Nikula Signed-off-by: Ville Syrjälä Reviewed-by: Jani Nikula Link: https://patch.msgid.link/20260408155744.13326-2-ville.syrjala@linux.intel.com Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/display/intel_display.c | 20 +++++--------------- 1 file changed, 5 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 10b6c6fcb03f..ad2fe10b6b1f 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -5939,17 +5939,6 @@ static int intel_atomic_check_joiner(struct intel_atomic_state *state, return -EINVAL; } - /* - * The state copy logic assumes the primary crtc gets processed - * before the secondary crtc during the main compute_config loop. - * This works because the crtcs are created in pipe order, - * and the hardware requires primary pipe < secondary pipe as well. - * Should that change we need to rethink the logic. - */ - if (WARN_ON(drm_crtc_index(&primary_crtc->base) > - drm_crtc_index(&secondary_crtc->base))) - return -EINVAL; - drm_dbg_kms(display->drm, "[CRTC:%d:%s] Used as secondary for joiner primary [CRTC:%d:%s]\n", secondary_crtc->base.base.id, secondary_crtc->base.name, @@ -6327,9 +6316,7 @@ static int intel_atomic_check_config(struct intel_atomic_state *state, for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) { if (!intel_crtc_needs_modeset(new_crtc_state)) { - if (intel_crtc_is_joiner_secondary(new_crtc_state)) - copy_joiner_crtc_state_nomodeset(state, crtc); - else + if (!intel_crtc_is_joiner_secondary(new_crtc_state)) intel_crtc_copy_uapi_to_hw_state_nomodeset(state, crtc); continue; } @@ -6460,8 +6447,11 @@ int intel_atomic_check(struct drm_device *dev, goto fail; for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) { - if (!intel_crtc_needs_modeset(new_crtc_state)) + if (!intel_crtc_needs_modeset(new_crtc_state)) { + if (intel_crtc_is_joiner_secondary(new_crtc_state)) + copy_joiner_crtc_state_nomodeset(state, crtc); continue; + } if (intel_crtc_is_joiner_secondary(new_crtc_state)) { drm_WARN_ON(display->drm, new_crtc_state->uapi.enable); From f49499e635c18ead4ba6b573c63955c02056b840 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Wed, 8 Apr 2026 18:57:44 +0300 Subject: [PATCH 2/3] drm/i915: Walk crtcs in pipe order MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently our crtcs are registered in pipe order, and thus all the for_intel_crtc*() iterators walk the crtcs in pipe order. There are a bunch of places that more or less depend on that. Eg. during plane updates and such we want joined pipes to be processed back-to-back to give a better chance of an atomic update across the whole set. When we start to register crtcs in a different order we don't want to change the order in which the pipes get handled. Decouple the for_each_intel_crtc*() iterators from the crtc registration order by using a separate list which will be sorted by the pipe rather than the crtc index. We could probably use a simple array or something, but that would require some kind of extra iterator variable for the macros, and thus would require a lot more changes. Using a linked list keeps the fallout minimal. We can look at using a more optimal data structure later. I also added this extra junk to the atomic state iterators: "(__i) = drm_crtc_index(&(crtc)->base), (void)(__i)" even though the macro itself no longer needs the "__i" iterator. This in case the "__i" is used by the caller, and to avoid compiler warnings if it's completely unused now. v2: Flip the pipe comparison (Jani) Reviewed-by: Jani Nikula Signed-off-by: Ville Syrjälä Link: https://patch.msgid.link/20260408155744.13326-3-ville.syrjala@linux.intel.com Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/display/intel_crtc.c | 20 +++++ drivers/gpu/drm/i915/display/intel_display.h | 90 ++++++++----------- .../gpu/drm/i915/display/intel_display_core.h | 3 + .../drm/i915/display/intel_display_driver.c | 1 + .../drm/i915/display/intel_display_types.h | 1 + drivers/gpu/drm/xe/display/xe_display.c | 1 + 6 files changed, 64 insertions(+), 52 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_crtc.c b/drivers/gpu/drm/i915/display/intel_crtc.c index b8189cd5d864..c88a6810c49f 100644 --- a/drivers/gpu/drm/i915/display/intel_crtc.c +++ b/drivers/gpu/drm/i915/display/intel_crtc.c @@ -209,6 +209,8 @@ static struct intel_crtc *intel_crtc_alloc(void) crtc->base.state = &crtc_state->uapi; crtc->config = crtc_state; + INIT_LIST_HEAD(&crtc->pipe_head); + return crtc; } @@ -222,6 +224,8 @@ static void intel_crtc_destroy(struct drm_crtc *_crtc) { struct intel_crtc *crtc = to_intel_crtc(_crtc); + list_del(&crtc->pipe_head); + cpu_latency_qos_remove_request(&crtc->vblank_pm_qos); drm_crtc_cleanup(&crtc->base); @@ -308,6 +312,20 @@ static const struct drm_crtc_funcs i8xx_crtc_funcs = { .get_vblank_timestamp = intel_crtc_get_vblank_timestamp, }; +static void add_crtc_to_pipe_list(struct intel_display *display, struct intel_crtc *crtc) +{ + struct intel_crtc *iter; + + list_for_each_entry(iter, &display->pipe_list, pipe_head) { + if (crtc->pipe < iter->pipe) { + list_add_tail(&crtc->pipe_head, &iter->pipe_head); + return; + } + } + + list_add_tail(&crtc->pipe_head, &display->pipe_list); +} + static int __intel_crtc_init(struct intel_display *display, enum pipe pipe) { struct intel_plane *primary, *cursor; @@ -398,6 +416,8 @@ static int __intel_crtc_init(struct intel_display *display, enum pipe pipe) if (HAS_CASF(display) && crtc->num_scalers >= 2) drm_crtc_create_sharpness_strength_property(&crtc->base); + add_crtc_to_pipe_list(display, crtc); + return 0; fail: diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h index 552a59d19e0f..1e76a455d7c4 100644 --- a/drivers/gpu/drm/i915/display/intel_display.h +++ b/drivers/gpu/drm/i915/display/intel_display.h @@ -212,22 +212,23 @@ enum phy_fia { base.head) \ for_each_if((intel_plane)->pipe == (intel_crtc)->pipe) -#define for_each_intel_crtc(dev, intel_crtc) \ - list_for_each_entry(intel_crtc, \ - &(dev)->mode_config.crtc_list, \ - base.head) +#define for_each_intel_crtc(dev, crtc) \ + list_for_each_entry((crtc), \ + &to_intel_display(dev)->pipe_list, \ + pipe_head) -#define for_each_intel_crtc_in_pipe_mask(dev, intel_crtc, pipe_mask) \ - list_for_each_entry(intel_crtc, \ - &(dev)->mode_config.crtc_list, \ - base.head) \ - for_each_if((pipe_mask) & BIT(intel_crtc->pipe)) +#define for_each_intel_crtc_reverse(dev, crtc) \ + list_for_each_entry_reverse((crtc), \ + &to_intel_display(dev)->pipe_list, \ + pipe_head) -#define for_each_intel_crtc_in_pipe_mask_reverse(dev, intel_crtc, pipe_mask) \ - list_for_each_entry_reverse((intel_crtc), \ - &(dev)->mode_config.crtc_list, \ - base.head) \ - for_each_if((pipe_mask) & BIT((intel_crtc)->pipe)) +#define for_each_intel_crtc_in_pipe_mask(dev, crtc, pipe_mask) \ + for_each_intel_crtc((dev), (crtc)) \ + for_each_if((pipe_mask) & BIT((crtc)->pipe)) + +#define for_each_intel_crtc_in_pipe_mask_reverse(dev, crtc, pipe_mask) \ + for_each_intel_crtc_reverse((dev), (crtc)) \ + for_each_if((pipe_mask) & BIT((crtc)->pipe)) #define for_each_intel_encoder(dev, intel_encoder) \ list_for_each_entry(intel_encoder, \ @@ -269,14 +270,6 @@ enum phy_fia { (__i)++) \ for_each_if(plane) -#define for_each_old_intel_crtc_in_state(__state, crtc, old_crtc_state, __i) \ - for ((__i) = 0; \ - (__i) < (__state)->base.dev->mode_config.num_crtc && \ - ((crtc) = to_intel_crtc((__state)->base.crtcs[__i].ptr), \ - (old_crtc_state) = to_intel_crtc_state((__state)->base.crtcs[__i].old_state), 1); \ - (__i)++) \ - for_each_if(crtc) - #define for_each_new_intel_plane_in_state(__state, plane, new_plane_state, __i) \ for ((__i) = 0; \ (__i) < (__state)->base.dev->mode_config.num_total_plane && \ @@ -285,22 +278,6 @@ enum phy_fia { (__i)++) \ for_each_if(plane) -#define for_each_new_intel_crtc_in_state(__state, crtc, new_crtc_state, __i) \ - for ((__i) = 0; \ - (__i) < (__state)->base.dev->mode_config.num_crtc && \ - ((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_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 && \ @@ -310,23 +287,32 @@ enum phy_fia { (__i)++) \ for_each_if(plane) +#define for_each_old_intel_crtc_in_state(__state, crtc, old_crtc_state, __i) \ + for_each_intel_crtc((__state)->base.dev, (crtc)) \ + for_each_if(((__i) = drm_crtc_index(&(crtc)->base), (void)(__i), \ + (old_crtc_state) = intel_atomic_get_old_crtc_state((__state), (crtc)))) + +#define for_each_new_intel_crtc_in_state(__state, crtc, new_crtc_state, __i) \ + for_each_intel_crtc((__state)->base.dev, (crtc)) \ + for_each_if(((__i) = drm_crtc_index(&(crtc)->base), (void)(__i), \ + (new_crtc_state) = intel_atomic_get_new_crtc_state((__state), (crtc)))) + +#define for_each_new_intel_crtc_in_state_reverse(__state, crtc, new_crtc_state, __i) \ + for_each_intel_crtc_reverse((__state)->base.dev, (crtc)) \ + for_each_if(((__i) = drm_crtc_index(&(crtc)->base), (void)(__i), \ + (new_crtc_state) = intel_atomic_get_new_crtc_state((__state), (crtc)))) + #define for_each_oldnew_intel_crtc_in_state(__state, crtc, old_crtc_state, new_crtc_state, __i) \ - for ((__i) = 0; \ - (__i) < (__state)->base.dev->mode_config.num_crtc && \ - ((crtc) = to_intel_crtc((__state)->base.crtcs[__i].ptr), \ - (old_crtc_state) = to_intel_crtc_state((__state)->base.crtcs[__i].old_state), \ - (new_crtc_state) = to_intel_crtc_state((__state)->base.crtcs[__i].new_state), 1); \ - (__i)++) \ - for_each_if(crtc) + for_each_intel_crtc((__state)->base.dev, (crtc)) \ + for_each_if(((__i) = drm_crtc_index(&(crtc)->base), (void)(__i), \ + (old_crtc_state) = intel_atomic_get_old_crtc_state((__state), (crtc)), \ + (new_crtc_state) = intel_atomic_get_new_crtc_state((__state), (crtc)))) #define for_each_oldnew_intel_crtc_in_state_reverse(__state, crtc, old_crtc_state, 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), \ - (old_crtc_state) = to_intel_crtc_state((__state)->base.crtcs[__i].old_state), \ - (new_crtc_state) = to_intel_crtc_state((__state)->base.crtcs[__i].new_state), 1); \ - (__i)--) \ - for_each_if(crtc) + for_each_intel_crtc_reverse((__state)->base.dev, (crtc)) \ + for_each_if(((__i) = drm_crtc_index(&(crtc)->base), (void)(__i), \ + (old_crtc_state) = intel_atomic_get_old_crtc_state((__state), (crtc)), \ + (new_crtc_state) = intel_atomic_get_new_crtc_state((__state), (crtc)))) #define intel_atomic_crtc_state_for_each_plane_state( \ plane, plane_state, \ diff --git a/drivers/gpu/drm/i915/display/intel_display_core.h b/drivers/gpu/drm/i915/display/intel_display_core.h index d708d322aa85..d9baca2d5aaf 100644 --- a/drivers/gpu/drm/i915/display/intel_display_core.h +++ b/drivers/gpu/drm/i915/display/intel_display_core.h @@ -294,6 +294,9 @@ struct intel_display { /* Parent, or core, driver functions exposed to display */ const struct intel_display_parent_interface *parent; + /* list of all intel_crtcs sorted by pipe */ + struct list_head pipe_list; + /* Display functions */ struct { /* Top level crtc-ish functions */ diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.c b/drivers/gpu/drm/i915/display/intel_display_driver.c index 23bfecc983e8..9c2f7ad6c7b7 100644 --- a/drivers/gpu/drm/i915/display/intel_display_driver.c +++ b/drivers/gpu/drm/i915/display/intel_display_driver.c @@ -117,6 +117,7 @@ static void intel_mode_config_init(struct intel_display *display) drm_mode_config_init(display->drm); INIT_LIST_HEAD(&display->global.obj_list); + INIT_LIST_HEAD(&display->pipe_list); mode_config->min_width = 0; mode_config->min_height = 0; diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h index e2496db1642a..f6cd0a062090 100644 --- a/drivers/gpu/drm/i915/display/intel_display_types.h +++ b/drivers/gpu/drm/i915/display/intel_display_types.h @@ -1484,6 +1484,7 @@ struct intel_flipq { struct intel_crtc { struct drm_crtc base; + struct list_head pipe_head; enum pipe pipe; /* * Whether the crtc and the connected output pipeline is active. Implies diff --git a/drivers/gpu/drm/xe/display/xe_display.c b/drivers/gpu/drm/xe/display/xe_display.c index a0a4ddf3bb46..00dfa68af29a 100644 --- a/drivers/gpu/drm/xe/display/xe_display.c +++ b/drivers/gpu/drm/xe/display/xe_display.c @@ -21,6 +21,7 @@ #include "intel_audio.h" #include "intel_bw.h" #include "intel_display.h" +#include "intel_display_core.h" #include "intel_display_device.h" #include "intel_display_driver.h" #include "intel_display_irq.h" From 2c683e9b419328da3433a49f7c467da71aaf0469 Mon Sep 17 00:00:00 2001 From: Jani Nikula Date: Mon, 13 Apr 2026 11:16:09 +0300 Subject: [PATCH 3/3] drm/i915/display: change pipe allocation order for discrete platforms MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When big joiner is enabled, it reserves the adjacent pipe as the secondary pipe. This happens without the user space knowing, and subsequent attempts at using the CRTC with that pipe will fail. If the user space does not have a coping mechanism, i.e. trying another CRTC, this leads to a black screen. Try to reduce the impact of the problem on discrete platforms by mapping the CRTCs to pipes in order A, C, B, and D. If the user space reserves CRTCs in order, this should trick it to using pipes that are more likely to be available for and after joining. Limit this to discrete platforms, which have four pipes, and no eDP, a combination that should benefit the most with least drawbacks. Cc: Ville Syrjala Reviewed-by: Ville Syrjälä Link: https://patch.msgid.link/20260413081609.969342-1-jani.nikula@intel.com Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/display/intel_crtc.c | 29 ++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_crtc.c b/drivers/gpu/drm/i915/display/intel_crtc.c index c88a6810c49f..03de219f7a64 100644 --- a/drivers/gpu/drm/i915/display/intel_crtc.c +++ b/drivers/gpu/drm/i915/display/intel_crtc.c @@ -411,8 +411,6 @@ static int __intel_crtc_init(struct intel_display *display, enum pipe pipe) cpu_latency_qos_add_request(&crtc->vblank_pm_qos, PM_QOS_DEFAULT_VALUE); - drm_WARN_ON(display->drm, drm_crtc_index(&crtc->base) != crtc->pipe); - if (HAS_CASF(display) && crtc->num_scalers >= 2) drm_crtc_create_sharpness_strength_property(&crtc->base); @@ -426,6 +424,31 @@ static int __intel_crtc_init(struct intel_display *display, enum pipe pipe) return ret; } +#define HAS_PIPE(display, pipe) (DISPLAY_RUNTIME_INFO(display)->pipe_mask & BIT(pipe)) + +/* + * Expose the pipes in order A, C, B, D on discrete platforms to trick user + * space into using pipes that are more likely to be available for both a) user + * space if pipe B has been reserved for the joiner, and b) the joiner if pipe A + * doesn't need the joiner. + * + * Swap pipes B and C only if both are available i.e. not fused off. + */ +static enum pipe reorder_pipe(struct intel_display *display, enum pipe pipe) +{ + if (!display->platform.dgfx || !HAS_PIPE(display, PIPE_B) || !HAS_PIPE(display, PIPE_C)) + return pipe; + + switch (pipe) { + case PIPE_B: + return PIPE_C; + case PIPE_C: + return PIPE_B; + default: + return pipe; + } +} + int intel_crtc_init(struct intel_display *display) { enum pipe pipe; @@ -435,7 +458,7 @@ int intel_crtc_init(struct intel_display *display) INTEL_NUM_PIPES(display), str_plural(INTEL_NUM_PIPES(display))); for_each_pipe(display, pipe) { - ret = __intel_crtc_init(display, pipe); + ret = __intel_crtc_init(display, reorder_pipe(display, pipe)); if (ret) return ret; }