Re: [PATCH v3] drm/i915/display: change pipe allocation order for discrete platforms

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

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 <jani.nikula@intel.com>
> > 
> > ---
> > 
> > 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