Re: [PATCH] drm/client: check whether CRTC is active before waiting for vblank

[Maarten Lankhorst]

Hey,

Den 2026-05-25 kl. 14:30, skrev Ville Syrjälä:
> On Mon, May 25, 2026 at 10:13:57AM +0200, Maarten Lankhorst wrote:
>> Hey,
>>
>> Den 2026-05-22 kl. 21:39, skrev Ville Syrjälä:
>>> On Fri, May 22, 2026 at 03:43:26PM +0200, Thomas Zimmermann wrote:
>>>> Hi
>>>>
>>>> Am 22.05.26 um 15:28 schrieb Ville Syrjälä:
>>>> [...]
>>>>>>>> But why does your HW use CRTC 1 in the first place.
>>>>>>> Could be eg. the enabled outputs can't be driven with CRTC 0.
>>>>>>>
>>>>>>> I guess what you want to do is pick the first crtc from modesets[]
>>>>>>> which is enabled. Or perhaps even "pick the Nth enabled crtc from
>>>>>>> modesets[] based on the ioctl argument".
>>>>>> The enable-status of each CRTC could change later on, which might lead
>>>>>> to problems.
>>>>> Sound like a locking issue if someone is changing the configuration
>>>>> at the same time we're trying to do the vblank wait here.
>>>>
>>>> I mean that the connected outputs could change at a later point or we 
>>>> could have multiple CRTCs in use. Today, someone in #intel-gfx reported 
>>>> a problem with panning if multiple CRTCs are in use.
>>>>
>>>> Therefore picking a CRTC freely could be a problem. Let's say we 
>>>> configure modes from one CRTC, but later wait/pan/flush with another 
>>>> CRTC. I would not trust this to work correctly.
>>>>
>>>> Hence, my suggestion is to select a primary CRTC during the fbdev 
>>>> client's probe and use it for all later operations until the next probe 
>>>> happens.  All other CRTCs would mirror the primary one.
>>>
>>> Actual mirroring may not be possible due to different modes supported
>>> on each output. The whole multi-output fbdev thing in the drm fb helper
>>> is kind of a hack that's rather hard to make work 100% sensibly.
>>>
>>> For the panning possibly the only sensible thing is to use the max of
>>> hdisplay/vdisplay of all the crtcs as the xres/yres so it's clear
>>> how much things can actually be panned. Oh and tiled displays (assuming
>>> we would actually want the fbdev stuff to tile correctly) make the
>>> situation even more complicated. I think the current support for tiled
>>> displays in the fb helper is semi-busted.´
>>
>> I tested fbdev on a tiled DP-MST monitor.
>> It works better than my kwin's wayland compositor, as it detects both tiles
>> and presents a single image spanning both tiles.
> 
> IIRC we've occasionally seen cases where it picks a non-tiled mode
> on the primary connector, and also still enables the second tile.
> 
>> Kwin sees both as separate
>> monitors.
>>
>> I still see vertical tearing between both tiles, so it would be nice if
>> intel/display would support atomic updates for both crtc's directly.
>>
>> The code's already there for bigjoiner, just needs to do the same for
>> tile joiner updates when all tiled crtc's are in the atomic update.
> 
> We don't have any special code for atomic updates with joiner
> currently. It just happens to work most of the time.
> 
> With joiner the pipes will be in sync/phase, so that helps a bit.
> But we do also try to make the pipes in sync/phase also for tiled
> display via the use of the port sync. So if you see a difference
> in tearing between joiner vs. tiled then that likely means the
> problem is in userspace (as in it submits separate commits for
> each tile).

The only thing kernel should do is if crtc 1 and 2 are part of a
commit and in sync is to perform the same as bigjoiner updates for
crtc 1 & 2 simultaneously.

That shouldn't be too much to ask from the kernel, and not hard to
implement.


> I have occasionally pondered about hiding the tiled display stuff
> completely from userspace and handling it in the kernel the same
> was as joiner. But the problem is that we'd also need to cook up
> a new EDID for the display that combines both tiles, and we'd
> still need the second connector to be enabled internally (and 
> hide that fact from userspace). None of that code exists
> currently and wouldn't be entirely trivial to implement.
>