Re: [PATCH v2] drm/i915/display: change pipe order for platforms with big joiner

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

On Tue, 18 Nov 2025, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Tue, Nov 18, 2025 at 07:40:06PM +0200, Ville Syrjälä wrote:
>> On Tue, Nov 18, 2025 at 03:43:35PM +0200, Jani Nikula wrote:
>> > On Tue, 18 Nov 2025, Jani Nikula <jani.nikula@intel.com> 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 pipe,
>> > > this leads to a black screen.
>> > >
>> > > If the platform allows joining A+B, map the CRTCs to pipes in order A,
>> > > C, B, and D to trick userspace to using pipes that are more likely to be
>> > > available for joining.
>> > >
>> > > 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.
>> > >
>> > > v2: Also remove WARN_ON()
>> > 
>> > There's still this in intel_atomic_check_joiner():
>> > 
>> > 		/*
>> > 		 * 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;
>> > 
>> > This still works for A+B and C+D joining, but will fail loudly for B+C
>> > joining.
>> > 
>> > Ideas?
>> 
>> Hmm, I think I got rid of that requirement semi-accidentally in
>> commit 3a5e09d82f97 ("drm/i915: Fix intel_modeset_pipe_config_late() for
>> bigjoiner")
>> 
>> So it looks to me like we can just drop that check entirely.
>
> After some pondering we might have a bit of problem with 
> the for_each_intel_crtc*() stuff. At least during
> the hardware programming phase we'd like those to walk the
> crtcs in pipe order but currently they use the crtc index
> order.
>
> Say we have a commit with pipes A+B,C. Currently we do
> everything in the C->B->A or A->B->C order. Thus the joined
> pipe A+B are always updated back-to-back, giving us the best
> chance to get an atomic update across them. With the shuffling
> pipe C will now sneak in between A and B, making it less likely
> that the updates on A and B will happen on the same frame.
>
> So I'm thinking we probably want to change all the 
> for_each_intel_crtc*() macros to use the pipe order as well.
> Either that or we need to review each use case and rewrite
> the important ones to use the pipe order.

Could this be the cause for [1]?

BR,
Jani.


[1] https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_157704v2/bat-twl-2/igt@i915_module_load@load.html#dmesg-warnings352


-- 
Jani Nikula, Intel