Re: [Nouveau] [PATCH] drm/nouveau: handle same-fb page flips

[Ben Skeggs]

On Fri, Oct 05, 2012 at 09:37:59PM +0200, Marcin Slusarz wrote:
 It's questionable use case, but weston/wayland already relies on this 
 behaviour,
 and other drivers don't care about it, so it's a matter of compatibility.
 Without it, process invoking such page flip hangs in unkillable state, trying
 to reserve the same buffer twice.
 
 Signed-off-by: Marcin Slusarz marcin.slus...@gmail.com
 ---
  drivers/gpu/drm/nouveau/nouveau_display.c | 20 +++-
  1 file changed, 15 insertions(+), 5 deletions(-)
 
 diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c 
 b/drivers/gpu/drm/nouveau/nouveau_display.c
 index 61f370d..a52cfd3 100644
 --- a/drivers/gpu/drm/nouveau/nouveau_display.c
 +++ b/drivers/gpu/drm/nouveau/nouveau_display.c
 @@ -530,9 +530,11 @@ nouveau_page_flip_reserve(struct nouveau_bo *old_bo,
   if (ret)
   goto fail;
  
 - ret = ttm_bo_reserve(old_bo-bo, false, false, false, 0);
 - if (ret)
 - goto fail_unreserve;
 + if (likely(old_bo != new_bo)) {
 + ret = ttm_bo_reserve(old_bo-bo, false, false, false, 0);
 + if (ret)
 + goto fail_unreserve;
 + }
  
   return 0;
  
 @@ -551,8 +553,10 @@ nouveau_page_flip_unreserve(struct nouveau_bo *old_bo,
   nouveau_bo_fence(new_bo, fence);
   ttm_bo_unreserve(new_bo-bo);
  
 - nouveau_bo_fence(old_bo, fence);
 - ttm_bo_unreserve(old_bo-bo);
 + if (likely(old_bo != new_bo)) {
 + nouveau_bo_fence(old_bo, fence);
 + ttm_bo_unreserve(old_bo-bo);
 + }
  
   nouveau_bo_unpin(old_bo);
  }
 @@ -624,6 +628,12 @@ nouveau_crtc_page_flip(struct drm_crtc *crtc, struct 
 drm_framebuffer *fb,
   if (!drm-channel)
   return -ENODEV;
  
 + if (unlikely(old_bo == new_bo)) {
 + char name[sizeof(current-comm)];
 + pr_debug_ratelimited('%s': useless page flip invoked\n,
 + get_task_comm(name, current));
 + }
 +
The patch looks alright, except I think we should just drop this hunk,
no other driver reports it either.

Thoughts?

Ben.

   s = kzalloc(sizeof(*s), GFP_KERNEL);
   if (!s)
   return -ENOMEM;
 -- 
 1.7.12
 
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [Nouveau] [PATCH] drm/nouveau: handle same-fb page flips

[Marcin Ślusarz]

17 paź 2012 06:31, Ben Skeggs skeg...@gmail.com napisał(a):

 On Fri, Oct 05, 2012 at 09:37:59PM +0200, Marcin Slusarz wrote:
  It's questionable use case, but weston/wayland already relies on this
behaviour,
  and other drivers don't care about it, so it's a matter of
compatibility.
  Without it, process invoking such page flip hangs in unkillable state,
trying
  to reserve the same buffer twice.
 
  Signed-off-by: Marcin Slusarz marcin.slus...@gmail.com
  ---
   drivers/gpu/drm/nouveau/nouveau_display.c | 20 +++-
   1 file changed, 15 insertions(+), 5 deletions(-)
 
  diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c
b/drivers/gpu/drm/nouveau/nouveau_display.c
  index 61f370d..a52cfd3 100644
  --- a/drivers/gpu/drm/nouveau/nouveau_display.c
  +++ b/drivers/gpu/drm/nouveau/nouveau_display.c
  @@ -530,9 +530,11 @@ nouveau_page_flip_reserve(struct nouveau_bo
*old_bo,
if (ret)
goto fail;
 
  - ret = ttm_bo_reserve(old_bo-bo, false, false, false, 0);
  - if (ret)
  - goto fail_unreserve;
  + if (likely(old_bo != new_bo)) {
  + ret = ttm_bo_reserve(old_bo-bo, false, false, false, 0);
  + if (ret)
  + goto fail_unreserve;
  + }
 
return 0;
 
  @@ -551,8 +553,10 @@ nouveau_page_flip_unreserve(struct nouveau_bo
*old_bo,
nouveau_bo_fence(new_bo, fence);
ttm_bo_unreserve(new_bo-bo);
 
  - nouveau_bo_fence(old_bo, fence);
  - ttm_bo_unreserve(old_bo-bo);
  + if (likely(old_bo != new_bo)) {
  + nouveau_bo_fence(old_bo, fence);
  + ttm_bo_unreserve(old_bo-bo);
  + }
 
nouveau_bo_unpin(old_bo);
   }
  @@ -624,6 +628,12 @@ nouveau_crtc_page_flip(struct drm_crtc *crtc,
struct drm_framebuffer *fb,
if (!drm-channel)
return -ENODEV;
 
  + if (unlikely(old_bo == new_bo)) {
  + char name[sizeof(current-comm)];
  + pr_debug_ratelimited('%s': useless page flip invoked\n,
  + get_task_comm(name, current));
  + }
  +
 The patch looks alright, except I think we should just drop this hunk,
 no other driver reports it either.

 Thoughts?

It will be easier to debug buggy clients with this code in. With multiple
wayland compositors it's possible some of them might be buggy... But I
don't feel strongly about it. You can delete this hunk when applying.

Marcin
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [Nouveau] [PATCH] drm/nouveau: handle same-fb page flips

[Marcin Slusarz]

On Fri, Oct 05, 2012 at 09:37:59PM +0200, Marcin Slusarz wrote:
 It's questionable use case, but weston/wayland already relies on this 
 behaviour,
 and other drivers don't care about it, so it's a matter of compatibility.
 Without it, process invoking such page flip hangs in unkillable state, trying
 to reserve the same buffer twice.
 
 Signed-off-by: Marcin Slusarz marcin.slus...@gmail.com
 ---
  drivers/gpu/drm/nouveau/nouveau_display.c | 20 +++-
  1 file changed, 15 insertions(+), 5 deletions(-)
 
 diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c 
 b/drivers/gpu/drm/nouveau/nouveau_display.c
 index 61f370d..a52cfd3 100644
 --- a/drivers/gpu/drm/nouveau/nouveau_display.c
 +++ b/drivers/gpu/drm/nouveau/nouveau_display.c
 @@ -530,9 +530,11 @@ nouveau_page_flip_reserve(struct nouveau_bo *old_bo,
   if (ret)
   goto fail;
  
 - ret = ttm_bo_reserve(old_bo-bo, false, false, false, 0);
 - if (ret)
 - goto fail_unreserve;
 + if (likely(old_bo != new_bo)) {
 + ret = ttm_bo_reserve(old_bo-bo, false, false, false, 0);
 + if (ret)
 + goto fail_unreserve;
 + }
  
   return 0;
  
 @@ -551,8 +553,10 @@ nouveau_page_flip_unreserve(struct nouveau_bo *old_bo,
   nouveau_bo_fence(new_bo, fence);
   ttm_bo_unreserve(new_bo-bo);
  
 - nouveau_bo_fence(old_bo, fence);
 - ttm_bo_unreserve(old_bo-bo);
 + if (likely(old_bo != new_bo)) {
 + nouveau_bo_fence(old_bo, fence);
 + ttm_bo_unreserve(old_bo-bo);
 + }
  
   nouveau_bo_unpin(old_bo);
  }
 @@ -624,6 +628,12 @@ nouveau_crtc_page_flip(struct drm_crtc *crtc, struct 
 drm_framebuffer *fb,
   if (!drm-channel)
   return -ENODEV;
  
 + if (unlikely(old_bo == new_bo)) {
 + char name[sizeof(current-comm)];
 + pr_debug_ratelimited('%s': useless page flip invoked\n,
 + get_task_comm(name, current));
 + }
 +
   s = kzalloc(sizeof(*s), GFP_KERNEL);
   if (!s)
   return -ENOMEM;
 -- 

ping
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau