Re: [PATCH v3 05/16] drm/fb-helper: do a generic fb_setcmap helper in terms of crtc .gamma_set

From: Daniel Vetter
Date: Thu Jul 06 2017 - 01:55:44 EST


On Wed, Jul 5, 2017 at 7:50 PM, Peter Rosin <peda@xxxxxxxxxx> wrote:
>>> +retry:
>>> + ret = drm_modeset_lock_all_ctx(dev, &ctx);
>>
>> With atomic you don't need to grab locks, this is done behind the scenes
>> (as long as you handle the retry/backoff correctly). See the kerneldoc for
>> the various drm_atomic_get_*_state functions.
>
> It doesn't work if I remove it. What is the disconnect?

Good question, didn't spot this at first, but your backoff/retry logic
is proken. When typing drm_modeset_lock locking code please make sure
you've enabled both CONFIG_PROVE_LOCKING and
CONFIG_DEBUG_WW_MUTEX_SLOWPATH. Without these two it's really easy to
get this wrong. Please also read
https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#kms-locking
carefully plus all the kernel-doc of the various hooks. This stuff is
a really tricky locking scheme, it takes a while to understand it and
implement it correctly. Which is why all the locking magic is in
shared code and for normal drivers no need think about it. For the
fundamental algorithm, you can also check out the docs for w/w mutexes
at https://www.kernel.org/doc/Documentation/locking/ww-mutex-design.txt

Might also help to read a bunch of the other locking paths again, with
my patches there's a few just in drm_fbdev_helper.c. I'll leave you
with these snippets here since I think this is fun to learn, but when
you're stuck I'm happy to help learn.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch