Re: [Intel-gfx] [PATCH 0/4] drm/atomic: Lockless blocking commits

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Tue, Sep 20, 2022 at 10:34:15AM +0300, Pekka Paalanen wrote:
> On Fri, 16 Sep 2022 19:33:27 +0300
> Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> 
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > I've talked about making blocking commits lockless a few
> > times in the past, so here's finally an attempt at it.
> > The main benefit I see from this is that TEST_ONLY commits
> > no longer getting blocked on the mutexes by parallel blocking
> > commits.
> > 
> > I have a small test here that spools up two threads,
> > one does just TEST_ONLY commits in a loop, the other
> > does either blocking or non-blocking page flips. Results
> > came out as follows on a snb machine here:
> > 
> > test-only-vs-non-blocking:
> > -85319 TEST_ONLY commits in 2000000 usecs, 23 usecs / commit
> > +87144 TEST_ONLY commits in 2000006 usecs, 22 usecs / commit
> > 
> > test-only-vs-blocking:
> > -219 TEST_ONLY commits in 2001768 usecs, 9140 usecs / commit
> > +82442 TEST_ONLY commits in 2000011 usecs, 24 usecs / commit
> > 
> > Now, I have no idea if anyone actually cares about lack
> > of parallelism due to locked blocking commits or not. Hence
> > Cc'd some compositor folks as well. I guess this is more of
> > an RFC at this point.
> > 
> > Also curious to see if CI goes up in smoke or not...
> 
> Hi Ville,
> 
> thanks for thinking about this. If I understand correctly, the issue
> you are solving here happens only when a blocking commit is underway
> while TEST_ONLY commits are done. This can only happen if userspace
> does the blocking commits from one thread, while another thread is
> doing TEST_ONLY probing on the same DRM device. It is inconsequential
> whether the two threads target distinct CRTCs or same CRTCs.
> 
> If so, this is not a problem for Weston for two reasons:
> 
> - Weston is fundamentally single-threaded, so if it does use a blocking
>   commit, it's not going to do anything else at the same time.
> 
> - Weston practically always uses non-blocking commits.
> 
> I cannot imagine those two facts to change.

I figured that is likely the case. Thanks for confirming.

> 
> Ah, but there is a case: KMS leasing!
> 
> With leasing you have two processes poking distinct CRTCs on the same
> device at the same time. Even if Weston never blocks, an arbitrary
> leasing client might, and I presume that would then stall Weston's
> TEST_ONLY commits.
> 
> I believe working on optimising this could be useful for KMS leasing use
> cases, assuming lessees do blocking commits. I don't know if any do.

Hmm, yeah didn't even think about leasing. Never have really.

The other reason (one I already forgot) I had for this is
drm_private_obj which has its own lock embbedded inside now.
So currently you have to think hard before actually using one
so as to not make everything block on it. With the locks not
held so much maybe drm_private_obj might become more palatable
for some things.

Oh, I guess there might also be some internal commits (or commit
like things) happening in the driver in some cases, such as DP
link retraining. At least with i915 those currently happen with
the locks held, but maybe could also be made lockless. But I
admit that those should be exceedingly rare situations.

-- 
Ville Syrjälä
Intel