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

[Jani Nikula <jani.nikula@intel.com>]

On Wed, 19 Nov 2025, Lucas De Marchi <lucas.demarchi@intel.com> 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) {
>
> this would previously skip fused off pipes from
> __intel_display_device_info_runtime_init(). Now we are just
> going to err_mode_config if one of the (first 4) pipes are
> fused off.
>
> I think we should check DISPLAY_RUNTIME_INFO(__dev_priv)->pipe_mask
> inside your loop below or IMO it would avoid some redundancy to change
> that to loop twice with for_each_pipe_masked(), passing
> PIPE_A | PIPE_C on first iteration.

Good catch, thanks!

BR,
Jani.

>
> Lucas De Marchi
>
>>+	/*
>>+	 * 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) {
>>+		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);
>>+		}
>>+	}
>>+
>>+	for_each_pipe_masked(display, pipe, pipe_mask) {
>> 		ret = intel_crtc_init(display, pipe);
>> 		if (ret)
>> 			goto err_mode_config;
>>-- 
>>2.47.3
>>

-- 
Jani Nikula, Intel