[RFC PATCH v2 7/9] drm: atomic: factor out common out-fence operations

2016-10-27 Thread Brian Starkey 
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

[RFC PATCH v2 7/9] drm: atomic: factor out common out-fence operations

2016-10-26 Thread Gustavo Padovan 
2016-10-26 Brian Starkey :

> 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 
> ---
>  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

[RFC PATCH v2 7/9] drm: atomic: factor out common out-fence operations

2016-10-26 Thread Gustavo Padovan 
2016-10-26 Brian Starkey :

> 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 
> ---
>  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

[RFC PATCH v2 7/9] drm: atomic: factor out common out-fence operations

2016-10-26 Thread Daniel Vetter 
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 

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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[RFC PATCH v2 7/9] drm: atomic: factor out common out-fence operations

2016-10-26 Thread Brian Starkey 
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 
>---
> 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
>

[RFC PATCH v2 7/9] drm: atomic: factor out common out-fence operations

2016-10-26 Thread Brian Starkey 
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 
---
 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