Re: [PATCH 6/9] drm/atomic: better doc for implicit vs explicit fencing

[Eric Anholt <eric@anholt.net>]

[-- Attachment #1.1: Type: text/plain, Size: 3787 bytes --]

Daniel Vetter <daniel.vetter@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 <kraxel@redhat.com>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: Harry Wentland <harry.wentland@amd.com>
> Cc: Sinclair Yeh <syeh@vmware.com>
> Cc: Thomas Hellstrom <thellstrom@vmware.com>
> Cc: Gustavo Padovan <gustavo@padovan.org>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Sean Paul <seanpaul@chromium.org>
> Signed-off-by: Daniel Vetter <daniel.vetter@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.

> +	 *
> +	 * 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 <eric@anholt.net>

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel