On Tue, Nov 15, 2016 at 10:06:41PM +0900, Gustavo Padovan wrote:
> From: Gustavo Padovan <gustavo.padovan at collabora.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 at collabora.co.uk>
> Reviewed-by: Brian Starkey <brian.starkey at arm.com>
> Tested-by: Robert Foss <robert.foss at collabora.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

Reply via email to