Simon Ser
2023-Jun-21 22:13 UTC
[Nouveau] [PATCH] drm/nouveau/disp: use drm_kms_helper_connector_hotplug_event()
Hi Lyude! On Wednesday, June 21st, 2023 at 23:56, Lyude Paul <lyude at redhat.com> wrote:> > - if (changed) > > + if (changed == 1) > > + drm_kms_helper_connector_hotplug_event(first_changed_connector); > > + else if (changed > 0) > > drm_kms_helper_hotplug_event(dev); > > I'm curious if you think there might be an advantage to doing this per- > connector even with multiple connectors? Seems like we could do that if we > stored changed connectors as a bitmask.I believe firing off multiple per-connector uevents would be worse than a single per-device uevent because user-space would see intermediate states instead of an atomic change. Let's say I have 3 connectors connected, and different settings set in my compositor when 1, 2 or 3 screens are connected. Let's say an unplug disconnects 2 monitors at the same time. With a single uevent, the compositor refreshes the whole device state, and then applies the 1-screen settings. With 2 uevents (one for each connector), the compositor refreshes the first disconnected connector state, then applies the 2-screen settings, then refreshes the second disconnected connector state, then applies the 1-screen settings. Applying the 2-screen settings might cause an unnecessarily glitchy intermediate state, where windows move around, modesets are performed, user configured commands are run, just to un-do everything right after. I hope this makes sense, Simon
Lyude Paul
2023-Jul-07 21:27 UTC
[Nouveau] [PATCH] drm/nouveau/disp: use drm_kms_helper_connector_hotplug_event()
On Wed, 2023-06-21 at 22:13 +0000, Simon Ser wrote:> Hi Lyude! > > On Wednesday, June 21st, 2023 at 23:56, Lyude Paul <lyude at redhat.com> wrote: > > > > - if (changed) > > > + if (changed == 1) > > > + drm_kms_helper_connector_hotplug_event(first_changed_connector); > > > + else if (changed > 0) > > > drm_kms_helper_hotplug_event(dev); > > > > I'm curious if you think there might be an advantage to doing this per- > > connector even with multiple connectors? Seems like we could do that if we > > stored changed connectors as a bitmask. > > I believe firing off multiple per-connector uevents would be worse than a > single per-device uevent because user-space would see intermediate states > instead of an atomic change. > > Let's say I have 3 connectors connected, and different settings set in my > compositor when 1, 2 or 3 screens are connected. Let's say an unplug > disconnects 2 monitors at the same time. With a single uevent, the compositor > refreshes the whole device state, and then applies the 1-screen settings. With > 2 uevents (one for each connector), the compositor refreshes the first > disconnected connector state, then applies the 2-screen settings, then > refreshes the second disconnected connector state, then applies the 1-screen > settings. Applying the 2-screen settings might cause an unnecessarily glitchy > intermediate state, where windows move around, modesets are performed, user > configured commands are run, just to un-do everything right after. > > I hope this makes sense, >Hi! Seems fine to me :) Reviewed-by: Lyude Paul <lyude at redhat.com> Let me know if you need me to push it to drm-misc> Simon >-- Cheers, Lyude Paul (she/her) Software Engineer at Red Hat