Re: [PATCH RESEND v7 0/2] Pass down hot-plug CONNECTOR ID to user-space

[Nicolas Frattaroli <nicolas.frattaroli@collabora.com>]

On Friday, 17 April 2026 16:16:01 Central European Summer Time Ville Syrjälä wrote:
> On Fri, Apr 17, 2026 at 02:42:36PM +0200, Nicolas Frattaroli wrote:
> > On Friday, 17 April 2026 12:57:58 Central European Summer Time Ville Syrjälä wrote:
> > > On Fri, Apr 17, 2026 at 09:49:36AM +0200, Michel Dänzer wrote:
> > > > On 4/16/26 15:16, Julian Orth wrote:
> > > > > On Thu, Apr 16, 2026 at 9:46 AM Nicolas Frattaroli
> > > > > <nicolas.frattaroli@collabora.com> wrote:
> > > > >> On Wednesday, 15 April 2026 20:57:53 Central European Summer Time Julian Orth wrote:
> > > > >>> On Wed, Apr 15, 2026 at 8:19 PM Nicolas Frattaroli
> > > > >>> <nicolas.frattaroli@collabora.com> wrote:
> > > > >>>>
> > > > >>>> This series addresses a shortcoming whereby a hot plug event is sent
> > > > >>>> without it being passed the actual connector that caused it. This takes
> > > > >>>> into consideration both the polling path and the HPD (Hot Plug Detect)
> > > > >>>> path. It also adds support for the vkms driver (using ConfigFS) for
> > > > >>>> propagating the connector ID when changing the connector's status.
> > > > >>>>
> > > > >>>> The motivation is that user-space applications such as Weston would
> > > > >>>> previously receive non-connector-specific hotplug events, and then have
> > > > >>>> to figure out themselves which connector needs to have a modeset
> > > > >>>> executed on. This notably did not work when the hotplug events came in
> > > > >>>> too fast, resulting in Weston missing an on-off-on transition of a
> > > > >>>> connector, seeing that its state was unchanged from "on" so can't be the
> > > > >>>> one that was hotplugged, and skipping reinitialising it as it looks
> > > > >>>> through the other connectors that could've caused it.
> > > > >>>
> > > > >>> Have you considered adding a u64 serial number as a DRM connector
> > > > >>> property that is incremented every time the connector changes in some
> > > > >>> way? Userspace could then check this serial number to see if the
> > > > >>> connector has changed since the last time it queried the serial
> > > > >>> number.
> > > > >>
> > > > >> The connector internally already has an epoch_counter member which
> > > > >> could be used for this. However, for the particular thing this
> > > > >> series fixes, I don't think exposing it through the uAPI is necessary
> > > > >> or desirable. Sending hotplug events specific to the connector does
> > > > >> not need any additional handling on the userspace side as long as it
> > > > >> already listens to the per-connector hotplug events in order to
> > > > >> avoid the pitfall described in the cover letter.
> > > > > 
> > > > > I currently do not handle per-connector hotplug events. Instead,
> > > > > whenever I get a UDEV change event for a device, I re-fetch the entire
> > > > > kernel state for the device. That is
> > > > > 
> > > > > - DRM_IOCTL_MODE_GETRESOURCES
> > > > > - DRM_IOCTL_MODE_OBJ_GETPROPERTIES for each connector, crtc, plane
> > > > > - DRM_IOCTL_MODE_GETCONNECTOR for each connector
> > > > > - DRM_IOCTL_MODE_GETPROPERTY for each connector property
> > > > > - DRM_IOCTL_MODE_GETPROPBLOB for the EDID
> > > > > 
> > > > > Once I have the new state, I compare it against the desired compositor
> > > > > state and perform a modeset if necessary.
> > > > 
> > > > mutter is doing something similar as well.
> > > > 
> > > > 
> > > > Note that some are arguing a modeset is always required after a hotplug event, even if the state hasn't changed.
> > > > 
> > > > The most convincing argument I've seen is the scenario of a GPU reset, after which a modeset is required to light up the displays again.
> > > 
> > > GPU reset should relight the display on its own really. That's what
> > > i915 does, albeit somewhat badly at the moment.
> > > 
> > > > A hotplug event seems the only mechanism available for the kernel to request a modeset from the compositor. (The kernel may not be able to reliably do the modeset on its own, e.g. due to interactions with user-space atomic commits)
> > > 
> > > There's nothing preventing the kernel from doing extra atomic
> > > commits whenever it wants. But if you want to punt the thing to
> > > userspace then the kernel must set the link-status property to
> > > bad, and then fire the hotplug uevent.
> > > 
> > > > If this "modeset required after hotplug event" rule is confirmed, it means that after a hotplug event without connector ID, the compositor must do a modeset for all connectors.
> > > 
> > > Only for connectors where things changed, or the link-status shows bad.
> > > 
> > > > 
> > > > 
> > > > P.S. https://gitlab.freedesktop.org/drm/i915/kernel/-/work_items/14420#note_2984697 even argued that two modesets are required after a hotplug event, one which turns things off and another one which turns them on again. I don't agree with that though, a single modeset should suffice.
> > > 
> > > The actual argument is that you should not defer the hotplug
> > > handling when things get disconnected, mainly because of crap type-c
> > > firmware.
> > > 
> > > I think the userspace behaviour there was that you get a disconnected,
> > > defer processing it, and then you get a reconnect, and then decide that
> > > nothing actually changed and a modeset is not needed after all. That is
> > > not correct IMO. Clearly a ->disconnect->reconnect should count as a
> > > change in the connector's state, and a full modeset is thus required.
> > > The kernel can of course then decide that the full modeset is not
> > > actually required and skip the modeset part during the commit. But
> > > userspace cannot make that determination.
> > 
> > So just to loop around to the patches here: is sending per-connector
> > hotplug events not acceptable? Your review[1] on v5 indicated you had
> > a problem with the implementation, not a fundamental issue with the
> > behaviour the patch tried to change.
> 
> What I was saying is that we already have the epoch_counter. If
> there are gaps in the implementation then those should just be
> fixed. We don't want a second implementation of the same
> mechanism.

epoch_counter is explicitly documented as "used to detect any other changes
in connector, besides status". pending_hp is changed if the status changes.

If we use epoch_counter for this, I think we'll need to keep track of the
last epoch_counter an hpd was sent for each connector, so we wouldn't win
anything in reduced state being tracked.

> > 
> > > I suppose what we could maybe do there to force userspace's hand is set
> > > the link-status to bad already when the thing gets disconnected, and keep
> > > it like that until a full disable+re-enable cycle has been done. Then
> > > userspace could not think that the ->disconnect->reconnect is a NOP
> > > (which I still think is incorrect behaviour).
> > 
> > The documentation of drm_connector_set_link_status_property explicitly
> > states:
> > 
> >     * Note: Drivers cannot rely on userspace to support this property and
> >     * issue a modeset. As such, they may choose to handle issues (like
> >     * re-training a link) without userspace's intervention.
> > 
> > which I think conflicts with your suggestion.
> 
> In i915 we do the retraining in kernel. IIRC we only really use the bad
> link stuff once retraining has failed hard enough that userspace
> intervention is needed, ie. when we cannot reduce the link parameters
> anymore without userspace selecting a mode with a lower resolution.
> 
> I think some more recent drivers may have opted to forego all in kernel
> retraining and just punt it all to userspace. I think the link-status
> property has existed for long enough that any userspace that doesn't
> support it could simply be considered broken.

Is breaking userspace a better choice than sending per-connector hotplug
events rather than a single global hotplug event? I get where you're
coming from, in that userspace should not be trying to optimise away
modesets, as that's something the kernel can do in a race-free way. I'm
just not familiar enough with the landscape to know whether setting
link-status to bad will actually result in userspace doing a modeset,
and whether drivers in practice then optimise that modeset away if it
wasn't needed.

> Actually I know of a single "userspace" that to my knowledge doesn't
> implement the link-status stuff, and that is the in kernel fb helper.
> That should probably be remedied...

Kind regards,
Nicolas Frattaroli