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
