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

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

On Wed, Nov 19, 2025 at 03:09:20PM +0200, Jani Nikula wrote:
> 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

That seems to claim that someone used a vblank work already during
driver load, or the list is corrupted or something. I don't think
that should happen, so I guess someone needs to figure out where
that is coming from. Also I can't think of a reason why it would
only happen with the crts reordered...

-- 
Ville Syrjälä
Intel