From: Daniel Vetter <daniel.vetter@ffwll.ch>
Date: Thu, 5 Apr 2018 17:44:47 +0200
Subject: drm/gem-fb-helper: Always do implicit sync
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Git-commit: 9d54fcd5416d84a3ab6a0b58846cff88ce62a4d4
Patch-mainline: v4.19-rc1
References: FATE#326289 FATE#326079 FATE#326049 FATE#322398 FATE#326166
I've done a lot of history digging. The first signs of this
optimization was introduced in i915:
commit 25067bfc060d1a481584dcb51ef4b5680176ecb6
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 ea2c67bb4affa84080c616920f3899f123786e56
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 cf3a7e4ce08e6876cdcb80390876647f28a7cf8f
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: Eric Anholt <eric@anholt.net>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180405154449.23038-8-daniel.vetter@ffwll.ch
Acked-by: Petr Tesarik <ptesarik@suse.com>
---
drivers/gpu/drm/drm_gem_framebuffer_helper.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
+++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
@@ -253,7 +253,7 @@ int drm_gem_fb_prepare_fb(struct drm_pla
struct dma_buf *dma_buf;
struct dma_fence *fence;
- if (plane->state->fb == state->fb || !state->fb)
+ if (!state->fb)
return 0;
dma_buf = drm_gem_fb_get_obj(state->fb, 0)->dma_buf;