[Intel-gfx] [PATCH] drm/i915: Put all non-blocking modesets onto an ordered wq

[Daniel Vetter]

On Wed, Nov 22, 2017 at 1:25 PM, Daniel Vetter <daniel.vetter at ffwll.ch> wrote:
> On Mon, Nov 13, 2017 at 3:36 PM, Maarten Lankhorst
> <maarten.lankhorst at linux.intel.com> wrote:
>> Op 13-11-17 om 14:36 schreef Ville Syrjala:
>>> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
>>>
>>> We have plenty of global registers and whatnot programmed without
>>> any further locking by the modeset code. Currently non-bocking
>>> modesets are allowed to execute in parallel which could corrupt
>>> said registers.
>>>
>>> To avoid the problem let's run all non-blocking modesets on an
>>> ordered workqueue. We still put page flips etc. to system_unbound_wq
>>> allowing page flips on one pipe to execute in parallel with page flips
>>> or a modeset on a another pipe (assuming no known state is shared
>>> between them, at which point they would have been added to the same
>>> atomic commit and serialized that way).
>>>
>>> Blocking modesets are already serialized with each other by
>>> connection_mutex, and thus are safe. To serialize them with
>>> non-blocking modesets we just flush the workqueue before executing
>>> blocking modesets.
>>>
>>> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
>>> Cc: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
>>> Fixes: 94f050246b42 ("drm/i915: nonblocking commit")
>>> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
>> This patch won't really fix it, you could still have a blocking modeset in parallel to a nonblocking one. What would really be needed to fix those instances here?
>
> The idea was that anything touching anything global would grap all
> crtc state, and then we'd track those using the drm_crtc_commit stuff.
> Putting everything onto one queue also doesn't work because they're
> meant to somewhat overlap (plane cleanup is in the same work, but
> should/can overlap with the next update).
>
> Imo the right fix is to make sure we do add all the crtc states
> everywhere we touch something global. And if that doesn't scale, then
> modeset objects to track those bits.

Backtracking a lot here: What kind of global registers do you mean
here? I have a bit the feeling that in a bunch of cases the problem
would then also be that it's not really atomic when it matters how we
interleave the updates ...
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch