• Daniel Vetter's avatar
    drm/gem-fb-helper: Always do implicit sync · 9d54fcd5
    Daniel Vetter authored
    I've done a lot of history digging. The first signs of this
    optimization was introduced in i915:
    
    commit 25067bfc
    Author: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
    Date:   Wed Sep 10 12:03:17 2014 -0300
    
        drm/i915: pin sprite fb only if it changed
    
    without much justification. Pinning already pinned stuff is real cheap
    (it's just obj->pin_count++ really), and the missing implicit sync was
    entirely forgotten about it seems. It's at least not mentioned
    anywhere it the commit message.
    
    It was also promptly removed shortly afterwards in
    
    commit ea2c67bb
    Author: Matt Roper <matthew.d.roper@intel.com>
    Date:   Tue Dec 23 10:41:52 2014 -0800
    
        drm/i915: Move to atomic plane helpers (v9)
    
    again without really mentioning the side-effect that plane updates
    with the same fb now again obey implicit syncing.
    
    Note that this only ever applied to the plane_update hook, all other
    legacy entry points (set_base, page_flip) always obeyed implicit sync
    in the drm/i915 driver.
    
    The real source of this code here seems to be msm, copied to vc4, then
    copied to tinydrm. I've also tried to dig around in all available msm
    sources, but the corresponding check for fb != old_fb is present ever
    since the initial merge in
    
    commit cf3a7e4c
    Author: Rob Clark <robdclark@gmail.com>
    Date:   Sat Nov 8 13:21:06 2014 -0500
    
        drm/msm: atomic core bits
    
    The only older version I've found of msm atomic code predates the
    atomic helpers, and so didn't even use any of this. It also does not
    have a corresponding check (because it simply did no implicit sync at
    all).
    
    I've chatted with Rob on irc, and he didn't remember the reason for
    this either.
    
    Note we had epic amounts of fun with too much syncing against
    _vblank_, especially around cursor updates. But I don't ever
    discussing a need for less syncing against implicit fences.
    
    Also note that explicit fencing allows you to sidetrack all of this,
    at least for all the drivers correctly implemented using
    drm_atomic_set_fence_for_plane().
    
    Given that it seems to be an accident of history, and that big drivers
    like i915 (and also nouveau it seems, I didn't follow the
    amdgpu/radeon sync code to figure this out properly there) never have
    done it, let's remove this.
    
    Cc: Rob Clark <robdclark@gmail.com>
    Cc: Matt Roper <matthew.d.roper@intel.com>
    Cc: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
    Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
    Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
    Cc: Sean Paul <seanpaul@chromium.org>
    Cc: David Airlie <airlied@linux.ie>
    Cc: "Noralf Trønnes" <noralf@tronnes.org>
    Reviewed-by: default avatarEric Anholt <eric@anholt.net>
    Signed-off-by: default avatarDaniel Vetter <daniel.vetter@intel.com>
    Link: https://patchwork.freedesktop.org/patch/msgid/20180405154449.23038-8-daniel.vetter@ffwll.ch
    9d54fcd5
drm_gem_framebuffer_helper.c 9.8 KB