On Fri, Jan 17, 2025 at 11:40:15AM +0000, Simon Ser wrote: > On Friday, January 17th, 2025 at 12:32, Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> wrote: > > > > + * When used with atomic uAPI, one event will be delivered per CRTC included in > > > + * the atomic commit. A CRTC is included in an atomic commit if one of its > > > + * properties is set, or if a property is set on a connector or plane linked > > > + * via the CRTC_ID property to the CRTC. At least one CRTC must be included, > > > + * and all pulled in CRTCs must be either previously or newly powered on (in > > > + * other words, a powered off CRTC which stays off cannot be included in the > > > + * atomic commit). > > > > I don't understand all this stuff about powered off crtcs? If > > someone sucks in a powered off thing then things will generally > > work just fine. > > Not with the page-flip event flag: > > /* > * Reject event generation for when a CRTC is off and stays off. > * It wouldn't be hard to implement this, but userspace has a track > * record of happily burning through 100% cpu (or worse, crash) when the > * display pipe is suspended. To avoid all that fun just reject updates > * that ask for events since likely that indicates a bug in the > * compositor's drawing loop. This is consistent with the vblank IOCTL > * and legacy page_flip IOCTL which also reject service on a disabled > * pipe. > */ > if (new_crtc_state->event && > !new_crtc_state->active && !old_crtc_state->active) { > drm_dbg_atomic(crtc->dev, > "[CRTC:%d:%s] requesting event but off\n", > crtc->base.id, crtc->name); > return -EINVAL; > } OK, so we're only talking about userspace stuff here. The kernel can still pull stuff in without too many issues (apart from the one mentined below). > > > There is a bit of corner case with the way we internally complete > > the commits for disabled things (not just crtcs, but also planes > > and connectors) and that can apparently happen a bit later than > > the commit completion for the enabled things. That seems to be > > causing a bit of grief for sway which insists on adding all kinds > > of disabled planes to every commit: > > https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13410 > > Hm, interesting. So including an already-disabled cursor plane in a > commit may fail the next commit with EBUY? I expect a lot of user-space > to do this as well, e.g. Weston. We may need to think if we could move that internal fake commit completion earlier a bit. But I don't actually remeber the specifics why it was added (presumably some commit ordering vs. cleanup problem) so not sure if that's easily doable or not. -- Ville Syrjälä Intel