| Message ID | 1477472108-27222-8-git-send-email-brian.starkey@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Oct 26, 2016 at 09:55:06AM +0100, Brian Starkey wrote: > Some parts of setting up the CRTC out-fence can be re-used for > writeback out-fences. Factor this out into a separate function. > > Signed-off-by: Brian Starkey <brian.starkey@arm.com> Cc: Gustavo here pls, probably best if he reviews this one. -Daniel > --- > drivers/gpu/drm/drm_atomic.c | 64 +++++++++++++++++++++++++++--------------- > 1 file changed, 42 insertions(+), 22 deletions(-) > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > index f434f34..3f8fc97 100644 > --- a/drivers/gpu/drm/drm_atomic.c > +++ b/drivers/gpu/drm/drm_atomic.c > @@ -1693,37 +1693,46 @@ void drm_atomic_clean_old_fb(struct drm_device *dev, > } > EXPORT_SYMBOL(drm_atomic_clean_old_fb); > > -static int crtc_setup_out_fence(struct drm_crtc *crtc, > - struct drm_crtc_state *crtc_state, > - struct drm_out_fence_state *fence_state) > +static struct fence *get_crtc_fence(struct drm_crtc *crtc, > + struct drm_crtc_state *crtc_state) > { > struct fence *fence; > > - fence_state->fd = get_unused_fd_flags(O_CLOEXEC); > - if (fence_state->fd < 0) { > - return fence_state->fd; > - } > - > - fence_state->out_fence_ptr = crtc_state->out_fence_ptr; > - crtc_state->out_fence_ptr = NULL; > - > - if (put_user(fence_state->fd, fence_state->out_fence_ptr)) > - return -EFAULT; > - > fence = kzalloc(sizeof(*fence), GFP_KERNEL); > if (!fence) > - return -ENOMEM; > + return NULL; > > fence_init(fence, &drm_crtc_fence_ops, &crtc->fence_lock, > crtc->fence_context, ++crtc->fence_seqno); > > + crtc_state->event->base.fence = fence_get(fence); > + > + return fence; > +} > + > +static int setup_out_fence(struct drm_out_fence_state *fence_state, > + u64 __user *out_fence_ptr, > + struct fence *fence) > +{ > + int ret; > + > + DRM_DEBUG_ATOMIC("Putting out-fence %p into user ptr: %p\n", > + fence, out_fence_ptr); > + > + fence_state->fd = get_unused_fd_flags(O_CLOEXEC); > + if (fence_state->fd < 0) > + return fence_state->fd; > + > + ret = put_user(fence_state->fd, out_fence_ptr); > + if (ret) > + return ret; > + > + fence_state->out_fence_ptr = out_fence_ptr; > + > fence_state->sync_file = sync_file_create(fence); > - if(!fence_state->sync_file) { > - fence_put(fence); > + if (!fence_state->sync_file) > return -ENOMEM; > - } > > - crtc_state->event->base.fence = fence_get(fence); > return 0; > } > > @@ -1896,10 +1905,21 @@ retry: > } > > if (crtc_state->out_fence_ptr) { > - ret = crtc_setup_out_fence(crtc, crtc_state, > - &fence_state[fence_idx++]); > - if (ret) > + struct fence *fence = get_crtc_fence(crtc, crtc_state); > + if (!fence) { > + ret = -ENOMEM; > + goto out; > + } > + > + ret = setup_out_fence(&fence_state[fence_idx++], > + crtc_state->out_fence_ptr, > + fence); > + if (ret) { > + fence_put(fence); > goto out; > + } > + > + crtc_state->out_fence_ptr = NULL; > } > } > > -- > 1.7.9.5 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
Hi Gustavo, As Daniel rightly pointed out you would likely be interested in this patch. This is on-top of your v5 patch-set, with the bug-fixes I mentioned before. I haven't dropped the fence_get(), as I figured you're probably going to rebase your patches on top of the fence_get() in sync_file_create(), and then I will do the same. Thanks, Brian On Wed, Oct 26, 2016 at 09:55:06AM +0100, Brian Starkey wrote: >Some parts of setting up the CRTC out-fence can be re-used for >writeback out-fences. Factor this out into a separate function. > >Signed-off-by: Brian Starkey <brian.starkey@arm.com> >--- > drivers/gpu/drm/drm_atomic.c | 64 +++++++++++++++++++++++++++--------------- > 1 file changed, 42 insertions(+), 22 deletions(-) > >diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c >index f434f34..3f8fc97 100644 >--- a/drivers/gpu/drm/drm_atomic.c >+++ b/drivers/gpu/drm/drm_atomic.c >@@ -1693,37 +1693,46 @@ void drm_atomic_clean_old_fb(struct drm_device *dev, > } > EXPORT_SYMBOL(drm_atomic_clean_old_fb); > >-static int crtc_setup_out_fence(struct drm_crtc *crtc, >- struct drm_crtc_state *crtc_state, >- struct drm_out_fence_state *fence_state) >+static struct fence *get_crtc_fence(struct drm_crtc *crtc, >+ struct drm_crtc_state *crtc_state) > { > struct fence *fence; > >- fence_state->fd = get_unused_fd_flags(O_CLOEXEC); >- if (fence_state->fd < 0) { >- return fence_state->fd; >- } >- >- fence_state->out_fence_ptr = crtc_state->out_fence_ptr; >- crtc_state->out_fence_ptr = NULL; >- >- if (put_user(fence_state->fd, fence_state->out_fence_ptr)) >- return -EFAULT; >- > fence = kzalloc(sizeof(*fence), GFP_KERNEL); > if (!fence) >- return -ENOMEM; >+ return NULL; > > fence_init(fence, &drm_crtc_fence_ops, &crtc->fence_lock, > crtc->fence_context, ++crtc->fence_seqno); > >+ crtc_state->event->base.fence = fence_get(fence); >+ >+ return fence; >+} >+ >+static int setup_out_fence(struct drm_out_fence_state *fence_state, >+ u64 __user *out_fence_ptr, >+ struct fence *fence) >+{ >+ int ret; >+ >+ DRM_DEBUG_ATOMIC("Putting out-fence %p into user ptr: %p\n", >+ fence, out_fence_ptr); >+ >+ fence_state->fd = get_unused_fd_flags(O_CLOEXEC); >+ if (fence_state->fd < 0) >+ return fence_state->fd; >+ >+ ret = put_user(fence_state->fd, out_fence_ptr); >+ if (ret) >+ return ret; >+ >+ fence_state->out_fence_ptr = out_fence_ptr; >+ > fence_state->sync_file = sync_file_create(fence); >- if(!fence_state->sync_file) { >- fence_put(fence); >+ if (!fence_state->sync_file) > return -ENOMEM; >- } > >- crtc_state->event->base.fence = fence_get(fence); > return 0; > } > >@@ -1896,10 +1905,21 @@ retry: > } > > if (crtc_state->out_fence_ptr) { >- ret = crtc_setup_out_fence(crtc, crtc_state, >- &fence_state[fence_idx++]); >- if (ret) >+ struct fence *fence = get_crtc_fence(crtc, crtc_state); >+ if (!fence) { >+ ret = -ENOMEM; >+ goto out; >+ } >+ >+ ret = setup_out_fence(&fence_state[fence_idx++], >+ crtc_state->out_fence_ptr, >+ fence); >+ if (ret) { >+ fence_put(fence); > goto out; >+ } >+ >+ crtc_state->out_fence_ptr = NULL; > } > } > >-- >1.7.9.5 >
2016-10-26 Brian Starkey <brian.starkey@arm.com>: > Some parts of setting up the CRTC out-fence can be re-used for > writeback out-fences. Factor this out into a separate function. > > Signed-off-by: Brian Starkey <brian.starkey@arm.com> > --- > drivers/gpu/drm/drm_atomic.c | 64 +++++++++++++++++++++++++++--------------- > 1 file changed, 42 insertions(+), 22 deletions(-) > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > index f434f34..3f8fc97 100644 > --- a/drivers/gpu/drm/drm_atomic.c > +++ b/drivers/gpu/drm/drm_atomic.c > @@ -1693,37 +1693,46 @@ void drm_atomic_clean_old_fb(struct drm_device *dev, > } > EXPORT_SYMBOL(drm_atomic_clean_old_fb); > > -static int crtc_setup_out_fence(struct drm_crtc *crtc, > - struct drm_crtc_state *crtc_state, > - struct drm_out_fence_state *fence_state) > +static struct fence *get_crtc_fence(struct drm_crtc *crtc, > + struct drm_crtc_state *crtc_state) > { > struct fence *fence; > > - fence_state->fd = get_unused_fd_flags(O_CLOEXEC); > - if (fence_state->fd < 0) { > - return fence_state->fd; > - } > - > - fence_state->out_fence_ptr = crtc_state->out_fence_ptr; > - crtc_state->out_fence_ptr = NULL; > - > - if (put_user(fence_state->fd, fence_state->out_fence_ptr)) > - return -EFAULT; > - > fence = kzalloc(sizeof(*fence), GFP_KERNEL); > if (!fence) > - return -ENOMEM; > + return NULL; > > fence_init(fence, &drm_crtc_fence_ops, &crtc->fence_lock, > crtc->fence_context, ++crtc->fence_seqno); > > + crtc_state->event->base.fence = fence_get(fence); > + > + return fence; > +} > + > +static int setup_out_fence(struct drm_out_fence_state *fence_state, > + u64 __user *out_fence_ptr, > + struct fence *fence) > +{ > + int ret; > + > + DRM_DEBUG_ATOMIC("Putting out-fence %p into user ptr: %p\n", > + fence, out_fence_ptr); > + > + fence_state->fd = get_unused_fd_flags(O_CLOEXEC); > + if (fence_state->fd < 0) > + return fence_state->fd; > + > + ret = put_user(fence_state->fd, out_fence_ptr); > + if (ret) > + return ret; > + > + fence_state->out_fence_ptr = out_fence_ptr; > + > fence_state->sync_file = sync_file_create(fence); > - if(!fence_state->sync_file) { > - fence_put(fence); > + if (!fence_state->sync_file) > return -ENOMEM; > - } > > - crtc_state->event->base.fence = fence_get(fence); > return 0; > } > > @@ -1896,10 +1905,21 @@ retry: > } > > if (crtc_state->out_fence_ptr) { > - ret = crtc_setup_out_fence(crtc, crtc_state, > - &fence_state[fence_idx++]); > - if (ret) > + struct fence *fence = get_crtc_fence(crtc, crtc_state); > + if (!fence) { > + ret = -ENOMEM; > + goto out; > + } > + > + ret = setup_out_fence(&fence_state[fence_idx++], > + crtc_state->out_fence_ptr, > + fence); > + if (ret) { > + fence_put(fence); > goto out; > + } > + > + crtc_state->out_fence_ptr = NULL; Yes, this split makes sense. I can break crtc_setup_out_fence() in my out_fence patch already. Gustavo
2016-10-26 Brian Starkey <brian.starkey@arm.com>: > Some parts of setting up the CRTC out-fence can be re-used for > writeback out-fences. Factor this out into a separate function. > > Signed-off-by: Brian Starkey <brian.starkey@arm.com> > --- > drivers/gpu/drm/drm_atomic.c | 64 +++++++++++++++++++++++++++--------------- > 1 file changed, 42 insertions(+), 22 deletions(-) > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > index f434f34..3f8fc97 100644 > --- a/drivers/gpu/drm/drm_atomic.c > +++ b/drivers/gpu/drm/drm_atomic.c > @@ -1693,37 +1693,46 @@ void drm_atomic_clean_old_fb(struct drm_device *dev, > } > EXPORT_SYMBOL(drm_atomic_clean_old_fb); > > -static int crtc_setup_out_fence(struct drm_crtc *crtc, > - struct drm_crtc_state *crtc_state, > - struct drm_out_fence_state *fence_state) > +static struct fence *get_crtc_fence(struct drm_crtc *crtc, > + struct drm_crtc_state *crtc_state) > { > struct fence *fence; > > - fence_state->fd = get_unused_fd_flags(O_CLOEXEC); > - if (fence_state->fd < 0) { > - return fence_state->fd; > - } > - > - fence_state->out_fence_ptr = crtc_state->out_fence_ptr; > - crtc_state->out_fence_ptr = NULL; > - > - if (put_user(fence_state->fd, fence_state->out_fence_ptr)) > - return -EFAULT; > - > fence = kzalloc(sizeof(*fence), GFP_KERNEL); > if (!fence) > - return -ENOMEM; > + return NULL; > > fence_init(fence, &drm_crtc_fence_ops, &crtc->fence_lock, > crtc->fence_context, ++crtc->fence_seqno); > > + crtc_state->event->base.fence = fence_get(fence); > + > + return fence; > +} > + > +static int setup_out_fence(struct drm_out_fence_state *fence_state, > + u64 __user *out_fence_ptr, > + struct fence *fence) > +{ > + int ret; > + > + DRM_DEBUG_ATOMIC("Putting out-fence %p into user ptr: %p\n", > + fence, out_fence_ptr); %p should be kept for your internal debug only. Make sure to remove anything that exposes kernel address when sending patches. Gustavo
On Wed, Oct 26, 2016 at 07:45:14PM -0200, Gustavo Padovan wrote: > >%p should be kept for your internal debug only. Make sure to remove >anything that exposes kernel address when sending patches. > >Gustavo > Noted, thanks! -Brian
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index f434f34..3f8fc97 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -1693,37 +1693,46 @@ void drm_atomic_clean_old_fb(struct drm_device *dev, } EXPORT_SYMBOL(drm_atomic_clean_old_fb); -static int crtc_setup_out_fence(struct drm_crtc *crtc, - struct drm_crtc_state *crtc_state, - struct drm_out_fence_state *fence_state) +static struct fence *get_crtc_fence(struct drm_crtc *crtc, + struct drm_crtc_state *crtc_state) { struct fence *fence; - fence_state->fd = get_unused_fd_flags(O_CLOEXEC); - if (fence_state->fd < 0) { - return fence_state->fd; - } - - fence_state->out_fence_ptr = crtc_state->out_fence_ptr; - crtc_state->out_fence_ptr = NULL; - - if (put_user(fence_state->fd, fence_state->out_fence_ptr)) - return -EFAULT; - fence = kzalloc(sizeof(*fence), GFP_KERNEL); if (!fence) - return -ENOMEM; + return NULL; fence_init(fence, &drm_crtc_fence_ops, &crtc->fence_lock, crtc->fence_context, ++crtc->fence_seqno); + crtc_state->event->base.fence = fence_get(fence); + + return fence; +} + +static int setup_out_fence(struct drm_out_fence_state *fence_state, + u64 __user *out_fence_ptr, + struct fence *fence) +{ + int ret; + + DRM_DEBUG_ATOMIC("Putting out-fence %p into user ptr: %p\n", + fence, out_fence_ptr); + + fence_state->fd = get_unused_fd_flags(O_CLOEXEC); + if (fence_state->fd < 0) + return fence_state->fd; + + ret = put_user(fence_state->fd, out_fence_ptr); + if (ret) + return ret; + + fence_state->out_fence_ptr = out_fence_ptr; + fence_state->sync_file = sync_file_create(fence); - if(!fence_state->sync_file) { - fence_put(fence); + if (!fence_state->sync_file) return -ENOMEM; - } - crtc_state->event->base.fence = fence_get(fence); return 0; } @@ -1896,10 +1905,21 @@ retry: } if (crtc_state->out_fence_ptr) { - ret = crtc_setup_out_fence(crtc, crtc_state, - &fence_state[fence_idx++]); - if (ret) + struct fence *fence = get_crtc_fence(crtc, crtc_state); + if (!fence) { + ret = -ENOMEM; + goto out; + } + + ret = setup_out_fence(&fence_state[fence_idx++], + crtc_state->out_fence_ptr, + fence); + if (ret) { + fence_put(fence); goto out; + } + + crtc_state->out_fence_ptr = NULL; } }
Some parts of setting up the CRTC out-fence can be re-used for writeback out-fences. Factor this out into a separate function. Signed-off-by: Brian Starkey <brian.starkey@arm.com> --- drivers/gpu/drm/drm_atomic.c | 64 +++++++++++++++++++++++++++--------------- 1 file changed, 42 insertions(+), 22 deletions(-)