Re: Parallel modesets and private state objects broken, where to go with MST?

[Pekka Paalanen <ppaalanen@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 3641 bytes --]

On Mon, 14 Mar 2022 18:16:36 -0400
Lyude Paul <lyude@redhat.com> wrote:

> So, the actual problem: following a conversation with Daniel Vetter today I've
> gotten the impression that private modesetting objects are basically just
> broken with parallel modesets? I'm still wrapping my head around all of this
> honestly, but from what I've gathered: CRTC atomic infra knows how to do waits
> in the proper places for when other CRTCs need to be waited on to continue a
> modeset, but there's no such tracking with private objects. If I understand
> this correctly, that means that even if two CRTC modesets require pulling in
> the same private object state for the MST mgr: we're only provided a guarantee
> that the atomic checks pulling in that private object state won't
> concurrently. But when it comes to commits, it doesn't sound like there's any
> actual tracking for this and as such - two CRTC modesets which have both
> pulled in the MST private state object are not prevented from running
> concurrently.
> 
> This unfortunately throws an enormous wrench into the MST atomic conversion
> I've been working on - as I was under the understanding while writing the code
> for this that all objects in an atomic state are blocked from being used in
> any new atomic commits (not checks, as parallel checks should be fine in my
> case) until there's no commits active with said object pulled into the atomic
> state. I certainly am not aware of any way parallel modesetting could actually
> be supported on MST, so it's not really a feature we want to deal with at all
> besides stopping it from happening. This also unfortunately means that the
> current atomic modesetting code we have for MST is potentially broken,
> although I assume we've never hit any real world issues with it because of the
> non-atomic locking we currently have for the payload table.
> 
> So, Daniel had mentioned that supposedly you've been dealing with similar
> issues with VC4 and might have already made progress on coming up with ways to
> deal with it. If this is all correct, I'd definitely appreciate being able to
> take a look at your work on this to see how I can help move things forward.
> I've got a WIP of my atomic only MST branch as well:
> 
> https://gitlab.freedesktop.org/lyudess/linux/-/commits/wip/mst-atomic-only-v1
> 
> However it's very certainly broken right now (it compiles and I had thought it
> worked already, but I realized I totally forgot to come up with a way of doing
> bookkeeping for VC start slots atomically - which is what led me down this
> current rabbit hole), but it should at least give a general idea of what I'm
> trying to do.
> 
> Anyway, let me know what you think.

Hi Lyude,

as mentioned in IRC, I believe parallel atomic modesets on separate
CRTCs have very limited use, so serialising them in the kernel is good.
Any userspace that wants to reliably program more than one CRTC will
collect all CRTCs into the same atomic commit.

The reason is that userspace does TEST_ONLY commits first to see if
things will work, and then do the real commit. If some other commit
lands in between, then the test results are invalid and would need to
be re-done. So parallel modesets are a gamble at best.

However, parallel modesets are not just an attack vector, they can
happen accidentally through DRM leasing. With DRM leasing, the DRM
master gives access to some CRTC to another process, which will be
doing modesets of its own. These two processes will race each other,
not having any idea what the other one is doing or when.


Thanks,
pq

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]