• Thierry Reding's avatar
    drm/plane: Add optional ->atomic_disable() callback · 407b8bd9
    Thierry Reding authored
    In order to prevent drivers from having to perform the same checks over
    and over again, add an optional ->atomic_disable callback which the core
    calls under the right circumstances.
    
    v2: pass old state and detect edges to avoid calling ->atomic_disable on
    already disabled planes, remove redundant comment (Daniel Vetter)
    
    v3: rename helper to drm_atomic_plane_disabling() to clarify that it is
    checking for transitions, move helper to drm_atomic_helper.h, clarify
    check for !old_state and its relation to transitional helpers
    
    Here's an extract from some discussion rationalizing the behaviour (for
    a full version, see the reference below):
    
        > > Hm, thinking about this some more this will result in a slight difference
        > > in behaviour, at least when drivers just use the helper ->reset functions
        > > but don't disable everything:
        > > - With transitional helpers we assume we know nothing and call
        > >   ->atomic_disable.
        > > - With atomic old_state->crtc == NULL in the same situation right after
        > >   boot-up, but we asssume the plane is really off and _dont_ call
        > >   ->atomic_disable.
        > >
        > > Should we instead check for (old_state && old_state->crtc) and state that
        > > drivers need to make sure they don't have stuff hanging around?
        >
        > I don't think we can check for old_state because otherwise this will
        > always return false, whereas we really want it to force-disable planes
        > that could be on (lacking any more accurate information). For
        > transitional helpers anyway.
        >
        > For the atomic helpers, old_state will never be NULL, but I'd assume
        > that the driver would reconstruct the current state in ->reset().
    
        By the way, the reason for why old_state can be NULL with transitional
        helpers is the ordering of the steps in the atomic transition. Currently
        the Tegra patches do this (based on your blog post and the Exynos proto-
        type):
    
            1) atomic conversion, phase 1:
               - implement ->atomic_{check,update,disable}()
               - use drm_plane_helper_{update,disable}()
    
            2) atomic conversion, phase 2:
               - call drm_mode_config_reset() from ->load()
               - implement ->reset()
    
        That's only a partial list of what's done in these steps, but that's the
        only relevant pieces for why old_state is NULL.
    
        What happens is that without ->reset() implemented there won't be any
        initial state, hence plane->state (the old_state here) will be NULL the
        first time atomic state is applied.
    
        We could of course reorder the sequence such that drivers are required
        to hook up ->reset() before they can (or at the same as they) hook up
        the transitional helpers. We could add an appropriate WARN_ON to this
        helper to make that more obvious.
    
        However, that will not solve the problem because it only gets rid of the
        special case. We still don't know whether old_state->crtc == NULL is the
        current state or just the initial default.
    
        So no matter which way we do this, I don't see a way to get away without
        requiring specific semantics from drivers. They would be that:
    
            - drivers recreate the correct state in ->reset() so that
              old_state->crtc != NULL if the plane is really enabled
    
        or
    
            - drivers have to ensure that the real state in fact mirrors the
              initial default as encoded in the state (plane disabled)
    
    References: http://lists.freedesktop.org/archives/dri-devel/2015-January/075578.htmlReviewed-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
    Reviewed-by: default avatarGustavo Padovan <gustavo.padovan@collabora.co.uk>
    Signed-off-by: default avatarThierry Reding <treding@nvidia.com>
    407b8bd9
drm_atomic_helper.h 6.61 KB