On Fri, Apr 20, 2018 at 03:04:58PM -0700, Eric Anholt wrote:
> Daniel Vetter <daniel.vet...@ffwll.ch> 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 <kra...@redhat.com>
> > Cc: Alex Deucher <alexander.deuc...@amd.com>
> > Cc: Harry Wentland <harry.wentl...@amd.com>
> > Cc: Sinclair Yeh <s...@vmware.com>
> > Cc: Thomas Hellstrom <thellst...@vmware.com>
> > Cc: Gustavo Padovan <gust...@padovan.org>
> > Cc: Maarten Lankhorst <maarten.lankho...@linux.intel.com>
> > Cc: Sean Paul <seanp...@chromium.org>
> > Signed-off-by: Daniel Vetter <daniel.vet...@intel.com>
> > ---
> > 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.
Yeah, that's better.
>
> > + *
> > + * 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?
Both suggestions implemented while applying, thanks very much for your
review.
-Daniel
>
> > + * &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 <e...@anholt.net>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
