Messages in this thread | | | Subject | Re: [PATCH v3 05/16] drm/fb-helper: do a generic fb_setcmap helper in terms of crtc .gamma_set | From | Peter Rosin <> | Date | Thu, 6 Jul 2017 08:18:02 +0200 |
| |
On 2017-07-06 07:55, Daniel Vetter wrote: > On Wed, Jul 5, 2017 at 7:50 PM, Peter Rosin <peda@axentia.se> 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,
Duh, for symmetry I also removed the dropping of the locks. So the next call of course had no chance of getting access. How silly of me...
> 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.
I'll take a long look at this before I send a cleaned up v4. Thanks for the pointers...
Cheers, peda
| |