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

[Pekka Paalanen <ppaalanen@gmail.com>]

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

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.

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.


Thanks,
pq



> 
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Rob Clark <robdclark@gmail.com>
> Cc: Simon Ser <contact@emersion.fr>
> Cc: Pekka Paalanen <pekka.paalanen@collabora.com>
> Cc: Jonas Ådahl <jadahl@gmail.com>
> 
> Ville Syrjälä (4):
>   drm/atomic: Treat a nonblocking commit following a blocking commit as
>     blocking commit
>   drm/i915: Don't reuse commit_work for the cleanup
>   drm/atomic: Allow lockless blocking commits
>   drm/i915: Make blocking commits lockless
> 
>  drivers/gpu/drm/drm_atomic.c                  | 32 +++++++++++++++++--
>  drivers/gpu/drm/drm_atomic_helper.c           | 19 +++++++----
>  drivers/gpu/drm/drm_atomic_uapi.c             | 11 +++++--
>  drivers/gpu/drm/i915/display/intel_display.c  | 15 +++------
>  .../drm/i915/display/intel_display_types.h    |  1 +
>  include/drm/drm_atomic.h                      |  8 +++++
>  6 files changed, 64 insertions(+), 22 deletions(-)
> 


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