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 Jul 06 2017 - 02:18:23 EST

On 2017-07-06 07:55, Daniel Vetter wrote:
> 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,

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
> 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
> 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...