| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20161116091053.GD24981@nuc-i3427.alporthouse.com>
Date: Wed, 16 Nov 2016 09:10:53 +0000
From: Chris Wilson <chris@...is-wilson.co.uk>
To: Gustavo Padovan <gustavo@...ovan.org>
Cc: dri-devel@...ts.freedesktop.org, marcheu@...gle.com,
Daniel Stone <daniels@...labora.com>, seanpaul@...gle.com,
Daniel Vetter <daniel.vetter@...ll.ch>,
linux-kernel@...r.kernel.org, laurent.pinchart@...asonboard.com,
Gustavo Padovan <gustavo.padovan@...labora.co.uk>,
John Harrison <John.C.Harrison@...el.com>, m.chehab@...sung.com
Subject: Re: [PATCH v12 3/3] drm/fence: add out-fences support
On Tue, Nov 15, 2016 at 10:06:41PM +0900, Gustavo Padovan wrote:
> From: Gustavo Padovan <gustavo.padovan@...labora.co.uk>
>
> Support DRM out-fences by creating a sync_file with a fence for each CRTC
> that sets the OUT_FENCE_PTR property.
>
> We use the out_fence pointer received in the OUT_FENCE_PTR prop to send
> the sync_file fd back to userspace.
>
> The sync_file and fd are allocated/created before commit, but the
> fd_install operation only happens after we know that commit succeed.
>
> v2: Comment by Rob Clark:
> - Squash commit that adds DRM_MODE_ATOMIC_OUT_FENCE flag here.
>
> Comment by Daniel Vetter:
> - Add clean up code for out_fences
>
> v3: Comments by Daniel Vetter:
> - create DRM_MODE_ATOMIC_EVENT_MASK
> - userspace should fill out_fences_ptr with the crtc_ids for which
> it wants fences back.
>
> v4: Create OUT_FENCE_PTR properties and remove old approach.
>
> v5: Comments by Brian Starkey:
> - Remove extra fence_get() in atomic_ioctl()
> - Check ret before iterating on the crtc_state
> - check ret before fd_install
> - set fence_state to NULL at the beginning
> - check fence_state->out_fence_ptr before put_user()
> - change order of fput() and put_unused_fd() on failure
>
> - Add access_ok() check to the out_fence_ptr received
> - Rebase after fence -> dma_fence rename
> - Store out_fence_ptr in the drm_atomic_state
> - Split crtc_setup_out_fence()
> - return -1 as out_fence with TEST_ONLY flag
>
> v6: Comments by Daniel Vetter
> - Add prepare/unprepare_crtc_signaling()
> - move struct drm_out_fence_state to drm_atomic.c
> - mark get_crtc_fence() as static
>
> Comments by Brian Starkey
> - proper set fence_ptr fence_state array
> - isolate fence_idx increment
>
> - improve error handling
>
> v7: Comments by Daniel Vetter
> - remove prefix from internal functions
> - make out_fence_ptr an s64 pointer
> - degrade DRM_INFO to DRM_DEBUG_ATOMIC when put_user fail
> - fix doc issues
> - filter out OUT_FENCE_PTR == NULL and do not fail in this case
> - add complete_crtc_signalling()
> - krealloc fence_state on demand
>
> Comment by Brian Starkey
> - remove unused crtc_state arg from get_out_fence()
>
> v8: Comment by Brian Starkey
> - cancel events before check for !fence_state
> - convert a few lefovers u64 types for out_fence_ptr
> - fix memleak by assign fence_state earlier after realloc
> - proper accout num_fences in case of error
>
> v9: Comment by Brian Starkey
> - memset last position of fence_state after krealloc
> Comments by Sean Paul
> - pass install_fds in complete_crtc_signaling() instead of ret
>
> - put_user(-1, fence_ptr) when decoding props
>
> v10: Comment by Brian Starkey
> - remove unneeded num_fences increment on error path
> - kfree fence_state after installing fences fd
>
> v11: rebase against latest drm-misc
>
> Signed-off-by: Gustavo Padovan <gustavo.padovan@...labora.co.uk>
> Reviewed-by: Brian Starkey <brian.starkey@....com>
> Tested-by: Robert Foss <robert.foss@...labora.com>
> ---
> drivers/gpu/drm/drm_atomic.c | 241 +++++++++++++++++++++++++++++++++++--------
> drivers/gpu/drm/drm_crtc.c | 8 ++
> include/drm/drm_atomic.h | 1 +
> include/drm/drm_crtc.h | 6 ++
> 4 files changed, 211 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 3ad780a..d4a92a9 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -290,6 +290,23 @@ drm_atomic_get_crtc_state(struct drm_atomic_state *state,
> }
> EXPORT_SYMBOL(drm_atomic_get_crtc_state);
>
> +static void set_out_fence_for_crtc(struct drm_atomic_state *state,
> + struct drm_crtc *crtc, s64 __user *fence_ptr)
> +{
> + state->crtcs[drm_crtc_index(crtc)].out_fence_ptr = fence_ptr;
> +}
> +
> +static s64 __user * get_out_fence_for_crtc(struct drm_atomic_state *state,
> + struct drm_crtc *crtc)
> +{
> + s64 __user *fence_ptr;
> +
> + fence_ptr = state->crtcs[drm_crtc_index(crtc)].out_fence_ptr;
> + state->crtcs[drm_crtc_index(crtc)].out_fence_ptr = NULL;
> +
> + return fence_ptr;
> +}
> +
> /**
> * drm_atomic_set_mode_for_crtc - set mode for CRTC
> * @state: the CRTC whose incoming state to update
> @@ -494,6 +511,16 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
> &replaced);
> state->color_mgmt_changed |= replaced;
> return ret;
> + } else if (property == config->prop_out_fence_ptr) {
> + s64 __user *fence_ptr = u64_to_user_ptr(val);
> +
> + if (!fence_ptr)
> + return 0;
> +
> + if (put_user(-1, fence_ptr))
> + return -EFAULT;
> +
> + set_out_fence_for_crtc(state->state, crtc, fence_ptr);
> } else if (crtc->funcs->atomic_set_property)
> return crtc->funcs->atomic_set_property(crtc, state, property, val);
> else
> @@ -536,6 +563,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
> *val = (state->ctm) ? state->ctm->base.id : 0;
> else if (property == config->gamma_lut_property)
> *val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
> + else if (property == config->prop_out_fence_ptr)
> + *val = 0;
> else if (crtc->funcs->atomic_get_property)
> return crtc->funcs->atomic_get_property(crtc, state, property, val);
> else
> @@ -1664,11 +1693,9 @@ int drm_atomic_debugfs_init(struct drm_minor *minor)
> */
>
> static struct drm_pending_vblank_event *create_vblank_event(
> - struct drm_device *dev, struct drm_file *file_priv,
> - struct dma_fence *fence, uint64_t user_data)
> + struct drm_device *dev, uint64_t user_data)
> {
> struct drm_pending_vblank_event *e = NULL;
> - int ret;
>
> e = kzalloc(sizeof *e, GFP_KERNEL);
> if (!e)
> @@ -1678,17 +1705,6 @@ static struct drm_pending_vblank_event *create_vblank_event(
> e->event.base.length = sizeof(e->event);
> e->event.user_data = user_data;
>
> - if (file_priv) {
> - ret = drm_event_reserve_init(dev, file_priv, &e->base,
> - &e->event.base);
> - if (ret) {
> - kfree(e);
> - return NULL;
> - }
> - }
> -
> - e->base.fence = fence;
> -
> return e;
> }
>
> @@ -1793,6 +1809,165 @@ void drm_atomic_clean_old_fb(struct drm_device *dev,
> }
> EXPORT_SYMBOL(drm_atomic_clean_old_fb);
>
> +static struct dma_fence *get_crtc_fence(struct drm_crtc *crtc)
> +{
return drm_crtc_fence_create(crtc);
(or possibly, drm_crtc_fence_get(), drm_crtc_timeline_advance() or
somesuch if we need finer control over fence_seqno)
Or if you want to embed it,
struct our_fence *fence;
fence = kzalloc(sizeof(*fence), GFP_KERNEL);
if (!fence)
return NULL;
drm_crtc_fence_init(crtc, &fence->base);
return &fence->base;
Looks tidier than dumping all the fence construction here
> + dma_fence_init(fence, &drm_crtc_fence_ops, &crtc->fence_lock,
> + crtc->fence_context, ++crtc->fence_seqno);
--
Chris Wilson, Intel Open Source Technology Centre
Powered by blists - more mailing lists