On Wed, Nov 19, 2025 at 07:24:22PM +0200, Ville Syrjälä wrote:
> On Wed, Nov 19, 2025 at 05:33:21PM +0200, Jani Nikula wrote:
> > 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.
> >
> > Although there are currently no platforms with more than four pipes, add
> > a fallback for initializing the rest of the pipes to not miss them.
> >
> > Signed-off-by: Jani Nikula <[email protected]>
> >
> > ---
> >
> > v2: Also remove WARN_ON()
> >
> > v3: Limit to discrete
> >
> > There are a number of issues in IGT with assuming CRTC index == pipe, at
> > least with CRC and vblank waits. With them being used a lot in tests, we
> > won't get enough test coverage until they're fixed.
> > ---
> > drivers/gpu/drm/i915/display/intel_crtc.c | 2 --
> > .../drm/i915/display/intel_display_driver.c | 26 ++++++++++++++++++-
> > 2 files changed, 25 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_crtc.c
> > b/drivers/gpu/drm/i915/display/intel_crtc.c
> > index 9d2a23c96c61..11e58d07ddef 100644
> > --- a/drivers/gpu/drm/i915/display/intel_crtc.c
> > +++ b/drivers/gpu/drm/i915/display/intel_crtc.c
> > @@ -394,8 +394,6 @@ 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))
> > drm_crtc_create_sharpness_strength_property(&crtc->base);
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.c
> > b/drivers/gpu/drm/i915/display/intel_display_driver.c
> > index 7e000ba3e08b..b5c9cdf14a43 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_driver.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display_driver.c
> > @@ -452,6 +452,7 @@ bool intel_display_driver_check_access(struct
> > intel_display *display)
> > /* part #2: call after irq install, but before gem init */
> > int intel_display_driver_probe_nogem(struct intel_display *display)
> > {
> > + u8 pipe_mask = U8_MAX;
> > enum pipe pipe;
> > int ret;
> >
> > @@ -470,7 +471,30 @@ int intel_display_driver_probe_nogem(struct
> > intel_display *display)
> > INTEL_NUM_PIPES(display),
> > INTEL_NUM_PIPES(display) > 1 ? "s" : "");
> >
> > - for_each_pipe(display, 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.
> > + *
> > + * Fall back to normal initialization for the remaining pipes, if any.
> > + */
> > + if (HAS_BIGJOINER(display) && display->platform.dgfx) {
>
> uncompressed joiner is also a thing.
>
> > + enum pipe pipe_order[] = { PIPE_A, PIPE_C, PIPE_B, PIPE_D };
> > + int i;
> > +
> > + for (i = 0; i < ARRAY_SIZE(pipe_order); i++) {
> > + pipe = pipe_order[i];
> > +
> > + ret = intel_crtc_init(display, pipe);
> > + if (ret)
> > + goto err_mode_config;
> > +
> > + pipe_mask &= ~BIT(pipe);
> > + }
> > + }
>
> I was thinking it might be easier to just do the B<->C swap inside
> intel_crtc_init(). Kinda similar how we we already do the plane
> A<->B swap in intel_primary_plane_create(). But I guess the
> loop here would become a bit more confusing since it would have
> to iterate all possible pipes and not just the ones present in
> the runtime info pipe_mask.
Gave this a quick try and I don't think it looks all that bad.
Shrug.
diff --git a/drivers/gpu/drm/i915/display/intel_crtc.c
b/drivers/gpu/drm/i915/display/intel_crtc.c
index 7ebbde716238..9279c76216ca 100644
--- a/drivers/gpu/drm/i915/display/intel_crtc.c
+++ b/drivers/gpu/drm/i915/display/intel_crtc.c
@@ -326,6 +326,21 @@ static void add_crtc_to_pipe_list(struct intel_display
*display, struct intel_cr
list_add_tail(&crtc->pipe_head, &display->pipe_list);
}
+static enum pipe reorder_pipe(struct intel_display *display, enum pipe pipe)
+{
+ if (!display->platform.dgfx)
+ 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)
{
struct intel_plane *primary, *cursor;
@@ -333,6 +348,11 @@ int intel_crtc_init(struct intel_display *display, enum
pipe pipe)
struct intel_crtc *crtc;
int sprite, ret;
+ pipe = reorder_pipe(display, pipe);
+
+ if ((DISPLAY_RUNTIME_INFO(display)->pipe_mask & BIT(pipe)) == 0)
+ return 0;
+
crtc = intel_crtc_alloc();
if (IS_ERR(crtc))
return PTR_ERR(crtc);
diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.c
b/drivers/gpu/drm/i915/display/intel_display_driver.c
index 32726906e550..cd30c6f18bb5 100644
--- a/drivers/gpu/drm/i915/display/intel_display_driver.c
+++ b/drivers/gpu/drm/i915/display/intel_display_driver.c
@@ -471,7 +471,7 @@ int intel_display_driver_probe_nogem(struct intel_display
*display)
INTEL_NUM_PIPES(display),
INTEL_NUM_PIPES(display) > 1 ? "s" : "");
- for_each_pipe(display, pipe) {
+ for (pipe = PIPE_A; pipe < I915_MAX_PIPES; pipe++) {
ret = intel_crtc_init(display, pipe);
if (ret)
goto err_mode_config;
--
Ville Syrjälä
Intel
