[PATCH] drm/atomic: Perform blocking commits on workqueue

[Ray Strode]

hi,

On Thu, Sep 28, 2023 at 5:43 AM Michel Dänzer
<michel.daenzer at mailbox.org> wrote:
> >>> When it's really not desirable to account the CPU overhead to the
> >>> process initiating it then you probably rather want to use an non
> >>> blocking commit plus a dma_fence to wait for the work to end from userspace.
> >> Well, first I don't think that's very convenient. You're talking about
> >> a per-plane property, so there would need to be a separate file
> >> descriptor allocated for every plane, right? and user-space would have
> >> to block on all of them before proceeding?
>
> OUT_FENCE_PTR is a per-CRTC property, not per-plane.

Okay, sure.

> Also, at least in this particular case, a single sync file (not dma_fence) for any CRTC might suffice.

I don't see how we could rely on that given the provided api and
multitude of drivers. It might
work and then break randomly.

> >> Is the code really going to allocate a vblank_event when all the
> >> crtc's are disabled ? I guess it's possible, but that's
> >> counterintuitive to me. I really don't know. You're confident a set of
> >> fences will actually work?
> >
> > No when you disable everything there is of course no fence allocated.
>
> I don't see why not. "new_crtc_in_state" in this code means the atomic
> commit contains some state for the CRTC (such as setting the
> OUT_FENCE_PTR property), it could disable or leave it disabled though.
> I can't see any other code which would prevent this from working with a
> disabled CRTC, I could be missing something though.

So I'll concede it's possible it works, and the fact it's using a vblank
type event is just technical debt (though Christian says he doesn't think it
works, so I think it's possible it doesn't work as well.)

Having said that, this whole argument of "the dysfunctional blocking api
is actually functioning as designed, but don't use it, since it doesn't work,
use the non-blocking api instead, because maybe it might work or it might not,
not sure" is pretty non-convincing to me.

> Handling modesets asynchronously does seem desirable in mutter to me.

Sure, fine. That doesn't mean we shouldn't fix the blocking call to behave like
almost all other blocking system calls.

--Ray