From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eric Anholt Subject: Re: [PATCH 6/9] drm/atomic: better doc for implicit vs explicit fencing Date: Fri, 20 Apr 2018 15:04:58 -0700 Message-ID: <87efj9ilf9.fsf@anholt.net> References: <20180405154449.23038-1-daniel.vetter@ffwll.ch> <20180405154449.23038-7-daniel.vetter@ffwll.ch> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1965548744==" Return-path: Received: from anholt.net (anholt.net [50.246.234.109]) by gabe.freedesktop.org (Postfix) with ESMTP id 2B4676E284 for ; Fri, 20 Apr 2018 22:05:03 +0000 (UTC) In-Reply-To: <20180405154449.23038-7-daniel.vetter@ffwll.ch> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: DRI Development Cc: Alex Deucher , Daniel Vetter , Thomas Hellstrom , Gerd Hoffmann , Daniel Vetter List-Id: dri-devel@lists.freedesktop.org --===============1965548744== Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" --=-=-= Content-Type: text/plain Daniel Vetter writes: > Note that a pile of drivers don't seem to take implicit fencing into > account, or at least don't call drm_atoimc_set_fence_for_plane(). ^atomic > Cc'ing relevant people, or at least some. Some drivers also look like > they don't disable implicit fencing (e.g. amdgpu) because the explicit > fences and implicit fences are handled by entirely independent code > paths. > > I also wonder whether we shouldn't just make the recommended helpers > the default ones, since a lot of drivers don't bother to handle the > implicit fences at all it seems. The helpers won't blow up even for > non-GEM drivers or GEM drivers which don't fill out the gem bo > pointers in struct drm_framebuffer. > > Cc: Gerd Hoffmann > Cc: Alex Deucher > Cc: Harry Wentland > Cc: Sinclair Yeh > Cc: Thomas Hellstrom > Cc: Gustavo Padovan > Cc: Maarten Lankhorst > Cc: Sean Paul > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/drm_atomic.c | 8 ++++++++ > include/drm/drm_modeset_helper_vtables.h | 5 ++++- > include/drm/drm_plane.h | 7 ++++++- > 3 files changed, 18 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > index 7d25c42f22db..ec77afbda0c3 100644 > --- a/drivers/gpu/drm/drm_atomic.c > +++ b/drivers/gpu/drm/drm_atomic.c > @@ -1492,6 +1492,14 @@ EXPORT_SYMBOL(drm_atomic_set_fb_for_plane); > * Otherwise, if &drm_plane_state.fence is not set this function we just set it > * with the received implicit fence. In both cases this function consumes a > * reference for @fence. > + * > + * This way explicit fencing can be used to overrule implicit fencing, which is > + * important to make explicit fencing use-cases work: One example is using one > + * buffer for 2 screens with different refresh rates. Implicit fencing will > + * clamp rendering to the refresh rate of the slower screen, whereas explicit > + * fence allows 2 independent render and display loops on a single buffer. If a > + * driver allows obeys both implicit and explicit fences for plane updates, then > + * it will break all the benefits of explicit fencing. > */ Thanks for explaining why we should care about explicit fencing for display! I'd been trying and failing to generate a usecase. > diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h > index d6da26d66a4b..1e2622e33208 100644 > --- a/include/drm/drm_plane.h > +++ b/include/drm/drm_plane.h > @@ -80,7 +80,12 @@ struct drm_plane_state { > * @fence: > * > * Optional fence to wait for before scanning out @fb. Do not write this > - * directly, use drm_atomic_set_fence_for_plane() > + * directly, use drm_atomic_set_fence_for_plane(). The core atomic code > + * will set this when userspace is using explicit fencing. Optional suggestion: * Optional fence to wait for before scanning out @fb. The core * atomic code will set this when userspace is using explicit * fencing. Do not write this directly for a driver's implicit * fence, use drm_atomic_set_fence_for_plane() to ensure that * an explicit fence is preserved. > + * > + * Drivers should store any implicit fences in this from their Maybe s/fences/fence/ to make it more obvious that you can only attach one? > + * &drm_plane_helper.prepare_fb callback. See drm_gem_fb_prepare_fb() > + * and drm_gem_fb_simple_display_pipe_prepare_fb() for suitable helpers. > */ > struct dma_fence *fence; Regardless, Reviewed-by: Eric Anholt --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEE/JuuFDWp9/ZkuCBXtdYpNtH8nugFAlraZAoACgkQtdYpNtH8 nuhS2w//VBaVA2kQTdMSdYz/scN5wn+FkaNdhj5NvtIye8jLFAQ1ROz+WNnGVVUR JG71zWxGFBpF1VMxxnH9uQMcbunc1srvT/7pMrkbvOP/xWG9MLm1hazf4t/q2FyQ xuhbNF18ohhRX9ekHfP1ZZa+3weCzXyqeW4or3KPSaTHFDNzQUtgyHP8qpBV7uQa X8g9InF0gmwZJjK7/Vox1wJlmkM7B9UK+fHtwOd+9gVV79XYowR1E6nN7mhdwGbj XEMBtGwjLcWzJtm/DGYd2vNwX02b5LPZ8ZFjEdwVhOmt3TIrFzdIoZBSrWZmY6a0 zXePagDoGtBYY4X1Y9gH9Z8Iwbn7F/nikWNFVze1Y7o51tiWGDiVhOVIlx9y4H14 t5Bl3kLH8VS6z213MKrgPLwBpV0ySKkriGVpLiHH13yUHq+FaED7xTrjTRdv9P/4 biH+pMKVDr0PhKw/fBUfjfJoIP+SaNxTOUfJHysdQa7WBwaVjGY8SCICSHHIwTLT 2/ubSryPhMZom0RNakRWbVJ95Gg/8QVmsKRmfxOj2k54M+D9+RPIMekgMLnrnalI jYqEahORV+tvwKNhLY6uX8yl8HSlXCCX8Kda1iGnhvm1TFaGgAe3W4apTyrPhRAj es+h039QBNAEZZj5P4++Twsd8YYDckBadUu/3wdgi5tohOd3lng= =gcFf -----END PGP SIGNATURE----- --=-=-=-- --===============1965548744== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== --===============1965548744==--