[PATCH] drm/syncobj: Drop add/remove_callback from driver interface

[PATCH] drm/syncobj: Drop add/remove_callback from driver interface

[Daniel Vetter]

This is used for handling future fences. Currently no driver use
these, and I think given the new timeline fence proposed by KHR it
would be better to have a more abstract interface for future fences.
Could be something simple like a struct dma_future_fence plus a
function to add a callback or wait for the fence to materialize.

Then syncobj (and anything else really) could grow new functions to
expose these two drivers. Normal dma_fence would then keep the nice
guarantee that they will always signal (and through ordering, be
deadlock free). dma_future_fence would then be the tricky one.

This also fixes sphinx complaining about the kerneldoc.

Cc: Jason Ekstrand <jason@jlekstrand.net>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_syncobj.c | 15 ---------------
 include/drm/drm_syncobj.h     |  5 -----
 2 files changed, 20 deletions(-)

diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index adb3cb27d31e..3a8837c49639 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -120,14 +120,6 @@ static int drm_syncobj_fence_get_or_add_callback(struct drm_syncobj *syncobj,
 	return ret;
 }
 
-/**
- * drm_syncobj_add_callback - adds a callback to syncobj::cb_list
- * @syncobj: Sync object to which to add the callback
- * @cb: Callback to add
- * @func: Func to use when initializing the drm_syncobj_cb struct
- *
- * This adds a callback to be called next time the fence is replaced
- */
 void drm_syncobj_add_callback(struct drm_syncobj *syncobj,
 			      struct drm_syncobj_cb *cb,
 			      drm_syncobj_func_t func)
@@ -136,13 +128,7 @@ void drm_syncobj_add_callback(struct drm_syncobj *syncobj,
 	drm_syncobj_add_callback_locked(syncobj, cb, func);
 	spin_unlock(&syncobj->lock);
 }
-EXPORT_SYMBOL(drm_syncobj_add_callback);
 
-/**
- * drm_syncobj_add_callback - removes a callback to syncobj::cb_list
- * @syncobj: Sync object from which to remove the callback
- * @cb: Callback to remove
- */
 void drm_syncobj_remove_callback(struct drm_syncobj *syncobj,
 				 struct drm_syncobj_cb *cb)
 {
@@ -150,7 +136,6 @@ void drm_syncobj_remove_callback(struct drm_syncobj *syncobj,
 	list_del_init(&cb->node);
 	spin_unlock(&syncobj->lock);
 }
-EXPORT_SYMBOL(drm_syncobj_remove_callback);
 
 /**
  * drm_syncobj_replace_fence - replace fence in a sync object.
diff --git a/include/drm/drm_syncobj.h b/include/drm/drm_syncobj.h
index 3980602472c0..e419c79ba94d 100644
--- a/include/drm/drm_syncobj.h
+++ b/include/drm/drm_syncobj.h
@@ -131,11 +131,6 @@ drm_syncobj_fence_get(struct drm_syncobj *syncobj)
 
 struct drm_syncobj *drm_syncobj_find(struct drm_file *file_private,
 				     u32 handle);
-void drm_syncobj_add_callback(struct drm_syncobj *syncobj,
-			      struct drm_syncobj_cb *cb,
-			      drm_syncobj_func_t func);
-void drm_syncobj_remove_callback(struct drm_syncobj *syncobj,
-				 struct drm_syncobj_cb *cb);
 void drm_syncobj_replace_fence(struct drm_syncobj *syncobj,
 			       struct dma_fence *fence);
 int drm_syncobj_find_fence(struct drm_file *file_private,
-- 
2.18.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.SPARSE: warning for drm/syncobj: Drop add/remove_callback from driver interface

[Patchwork]

== Series Details ==

Series: drm/syncobj: Drop add/remove_callback from driver interface
URL   : https://patchwork.freedesktop.org/series/48542/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Commit: drm/syncobj: Drop add/remove_callback from driver interface
+drivers/gpu/drm/drm_syncobj.c:123:6: warning: symbol 'drm_syncobj_add_callback' was not declared. Should it be static?
+drivers/gpu/drm/drm_syncobj.c:132:6: warning: symbol 'drm_syncobj_remove_callback' was not declared. Should it be static?

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/syncobj: Drop add/remove_callback from driver interface

[Patchwork]

== Series Details ==

Series: drm/syncobj: Drop add/remove_callback from driver interface
URL   : https://patchwork.freedesktop.org/series/48542/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4695 -> Patchwork_9988 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/48542/revisions/1/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_9988:

  === IGT changes ===

    ==== Possible regressions ====

    {igt@pm_rpm@module-reload}:
      fi-byt-j1900:       NOTRUN -> DMESG-WARN

    
== Known issues ==

  Here are the changes found in Patchwork_9988 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    {igt@amdgpu/amd_basic@userptr}:
      {fi-kbl-8809g}:     PASS -> INCOMPLETE (fdo#107402)

    {igt@pm_rpm@module-reload}:
      fi-cnl-psr:         PASS -> WARN (fdo#107602)

    
    ==== Possible fixes ====

    igt@drv_selftest@live_hangcheck:
      {fi-cfl-8109u}:     DMESG-FAIL (fdo#106560) -> PASS

    
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  fdo#106560 https://bugs.freedesktop.org/show_bug.cgi?id=106560
  fdo#107402 https://bugs.freedesktop.org/show_bug.cgi?id=107402
  fdo#107602 https://bugs.freedesktop.org/show_bug.cgi?id=107602


== Participating hosts (52 -> 47) ==

  Additional (1): fi-byt-j1900 
  Missing    (6): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-bdw-samus 


== Build changes ==

    * Linux: CI_DRM_4695 -> Patchwork_9988

  CI_DRM_4695: f352837700ec39dbaf00cdf727650851993d754b @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4608: 94ebd21177feedf03e8f6dd1e73dca1a6ec7a0ac @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9988: 573052f7cbf140741e3689c403484e3182fe9f94 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

573052f7cbf1 drm/syncobj: Drop add/remove_callback from driver interface

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9988/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for drm/syncobj: Drop add/remove_callback from driver interface

[Patchwork]

== Series Details ==

Series: drm/syncobj: Drop add/remove_callback from driver interface
URL   : https://patchwork.freedesktop.org/series/48542/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4695_full -> Patchwork_9988_full =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_9988_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_9988_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_9988_full:

  === IGT changes ===

    ==== Warnings ====

    igt@gem_ctx_param@root-set-no-zeromap-disabled:
      shard-kbl:          PASS -> ( 2 PASS ) +541

    igt@gem_exec_big:
      shard-apl:          SKIP -> ( 2 SKIP ) +197

    igt@gem_mocs_settings@mocs-reset-bsd2:
      shard-snb:          SKIP -> ( 2 SKIP ) +343

    igt@gem_pread@stolen-uncached:
      shard-kbl:          SKIP -> ( 2 SKIP ) +207

    igt@gem_pwrite@small-cpu-random:
      shard-hsw:          PASS -> ( 2 PASS ) +452

    igt@kms_concurrent@pipe-d:
      shard-hsw:          SKIP -> ( 2 SKIP ) +239

    igt@kms_frontbuffer_tracking@psr-rgb101010-draw-pwrite:
      shard-glk:          SKIP -> ( 2 SKIP ) +173

    igt@kms_plane@plane-panning-bottom-right-pipe-a-planes:
      shard-glk:          PASS -> ( 2 PASS ) +440

    igt@kms_rotation_crc@sprite-rotation-90-pos-100-0:
      shard-apl:          PASS -> ( 2 PASS ) +416

    igt@syncobj_wait@invalid-wait-bad-flags:
      shard-snb:          PASS -> ( 2 PASS ) +348

    
== Known issues ==

  Here are the changes found in Patchwork_9988_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_ctx_isolation@vcs1-s3:
      shard-kbl:          PASS -> ( 2 INCOMPLETE ) (fdo#103665)

    igt@kms_setmode@basic:
      shard-apl:          PASS -> FAIL (fdo#99912)

    
    ==== Possible fixes ====

    igt@gem_ppgtt@blt-vs-render-ctx0:
      shard-kbl:          INCOMPLETE (fdo#103665, fdo#106023) -> PASS

    
    ==== Warnings ====

    igt@gem_exec_schedule@pi-ringfull-bsd1:
      shard-kbl:          FAIL (fdo#103158) -> ( 2 FAIL ) (fdo#103158) +1

    igt@gem_exec_schedule@pi-ringfull-render:
      shard-apl:          FAIL (fdo#103158) -> ( 2 FAIL ) (fdo#103158)

    igt@kms_ccs@pipe-b-crc-sprite-planes-basic:
      shard-apl:          FAIL (fdo#105458, fdo#106510) -> ( 2 FAIL ) (fdo#105458, fdo#106510)

    igt@kms_flip@flip-vs-expired-vblank:
      shard-glk:          FAIL (fdo#105363) -> ( 1 FAIL, 1 PASS ) (fdo#102887, fdo#105363)

    igt@kms_rotation_crc@primary-rotation-180:
      shard-snb:          FAIL (fdo#103925) -> ( 2 FAIL ) (fdo#103925)

    igt@kms_setmode@basic:
      shard-kbl:          FAIL (fdo#99912) -> ( 2 FAIL ) (fdo#99912)

    igt@kms_sysfs_edid_timing:
      shard-hsw:          WARN (fdo#100047) -> ( 2 WARN ) (fdo#100047)
      shard-glk:          WARN (fdo#100047) -> ( 2 WARN ) (fdo#100047)
      shard-kbl:          FAIL (fdo#100047) -> ( 2 FAIL ) (fdo#100047)

    
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047
  fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
  fdo#103158 https://bugs.freedesktop.org/show_bug.cgi?id=103158
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#103925 https://bugs.freedesktop.org/show_bug.cgi?id=103925
  fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
  fdo#105458 https://bugs.freedesktop.org/show_bug.cgi?id=105458
  fdo#106023 https://bugs.freedesktop.org/show_bug.cgi?id=106023
  fdo#106510 https://bugs.freedesktop.org/show_bug.cgi?id=106510
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


== Participating hosts (5 -> 5) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4695 -> Patchwork_9988

  CI_DRM_4695: f352837700ec39dbaf00cdf727650851993d754b @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4608: 94ebd21177feedf03e8f6dd1e73dca1a6ec7a0ac @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9988: 573052f7cbf140741e3689c403484e3182fe9f94 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9988/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/syncobj: Drop add/remove_callback from driver interface

[Jason Ekstrand]

[-- Attachment #1.1: Type: text/plain, Size: 3689 bytes --]

Fine with me.

Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>

On Wed, Aug 22, 2018 at 4:29 AM Daniel Vetter <daniel.vetter@ffwll.ch>
wrote:

> This is used for handling future fences. Currently no driver use
> these, and I think given the new timeline fence proposed by KHR it
> would be better to have a more abstract interface for future fences.
> Could be something simple like a struct dma_future_fence plus a
> function to add a callback or wait for the fence to materialize.
>
> Then syncobj (and anything else really) could grow new functions to
> expose these two drivers. Normal dma_fence would then keep the nice
> guarantee that they will always signal (and through ordering, be
> deadlock free). dma_future_fence would then be the tricky one.
>
> This also fixes sphinx complaining about the kerneldoc.
>
> Cc: Jason Ekstrand <jason@jlekstrand.net>
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  drivers/gpu/drm/drm_syncobj.c | 15 ---------------
>  include/drm/drm_syncobj.h     |  5 -----
>  2 files changed, 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> index adb3cb27d31e..3a8837c49639 100644
> --- a/drivers/gpu/drm/drm_syncobj.c
> +++ b/drivers/gpu/drm/drm_syncobj.c
> @@ -120,14 +120,6 @@ static int
> drm_syncobj_fence_get_or_add_callback(struct drm_syncobj *syncobj,
>         return ret;
>  }
>
> -/**
> - * drm_syncobj_add_callback - adds a callback to syncobj::cb_list
> - * @syncobj: Sync object to which to add the callback
> - * @cb: Callback to add
> - * @func: Func to use when initializing the drm_syncobj_cb struct
> - *
> - * This adds a callback to be called next time the fence is replaced
> - */
>  void drm_syncobj_add_callback(struct drm_syncobj *syncobj,
>                               struct drm_syncobj_cb *cb,
>                               drm_syncobj_func_t func)
> @@ -136,13 +128,7 @@ void drm_syncobj_add_callback(struct drm_syncobj
> *syncobj,
>         drm_syncobj_add_callback_locked(syncobj, cb, func);
>         spin_unlock(&syncobj->lock);
>  }
> -EXPORT_SYMBOL(drm_syncobj_add_callback);
>
> -/**
> - * drm_syncobj_add_callback - removes a callback to syncobj::cb_list
> - * @syncobj: Sync object from which to remove the callback
> - * @cb: Callback to remove
> - */
>  void drm_syncobj_remove_callback(struct drm_syncobj *syncobj,
>                                  struct drm_syncobj_cb *cb)
>  {
> @@ -150,7 +136,6 @@ void drm_syncobj_remove_callback(struct drm_syncobj
> *syncobj,
>         list_del_init(&cb->node);
>         spin_unlock(&syncobj->lock);
>  }
> -EXPORT_SYMBOL(drm_syncobj_remove_callback);
>
>  /**
>   * drm_syncobj_replace_fence - replace fence in a sync object.
> diff --git a/include/drm/drm_syncobj.h b/include/drm/drm_syncobj.h
> index 3980602472c0..e419c79ba94d 100644
> --- a/include/drm/drm_syncobj.h
> +++ b/include/drm/drm_syncobj.h
> @@ -131,11 +131,6 @@ drm_syncobj_fence_get(struct drm_syncobj *syncobj)
>
>  struct drm_syncobj *drm_syncobj_find(struct drm_file *file_private,
>                                      u32 handle);
> -void drm_syncobj_add_callback(struct drm_syncobj *syncobj,
> -                             struct drm_syncobj_cb *cb,
> -                             drm_syncobj_func_t func);
> -void drm_syncobj_remove_callback(struct drm_syncobj *syncobj,
> -                                struct drm_syncobj_cb *cb);
>  void drm_syncobj_replace_fence(struct drm_syncobj *syncobj,
>                                struct dma_fence *fence);
>  int drm_syncobj_find_fence(struct drm_file *file_private,
> --
> 2.18.0
>
>

[-- Attachment #1.2: Type: text/html, Size: 4716 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/syncobj: Drop add/remove_callback from driver interface

[Daniel Vetter]

On Wed, Aug 22, 2018 at 11:09:21AM -0500, Jason Ekstrand wrote:
> Fine with me.
> 
> Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>

Thanks for the review, applied to drm-misc-next.
-Daniel

> 
> On Wed, Aug 22, 2018 at 4:29 AM Daniel Vetter <daniel.vetter@ffwll.ch>
> wrote:
> 
> > This is used for handling future fences. Currently no driver use
> > these, and I think given the new timeline fence proposed by KHR it
> > would be better to have a more abstract interface for future fences.
> > Could be something simple like a struct dma_future_fence plus a
> > function to add a callback or wait for the fence to materialize.
> >
> > Then syncobj (and anything else really) could grow new functions to
> > expose these two drivers. Normal dma_fence would then keep the nice
> > guarantee that they will always signal (and through ordering, be
> > deadlock free). dma_future_fence would then be the tricky one.
> >
> > This also fixes sphinx complaining about the kerneldoc.
> >
> > Cc: Jason Ekstrand <jason@jlekstrand.net>
> > Cc: Dave Airlie <airlied@redhat.com>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > ---
> >  drivers/gpu/drm/drm_syncobj.c | 15 ---------------
> >  include/drm/drm_syncobj.h     |  5 -----
> >  2 files changed, 20 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> > index adb3cb27d31e..3a8837c49639 100644
> > --- a/drivers/gpu/drm/drm_syncobj.c
> > +++ b/drivers/gpu/drm/drm_syncobj.c
> > @@ -120,14 +120,6 @@ static int
> > drm_syncobj_fence_get_or_add_callback(struct drm_syncobj *syncobj,
> >         return ret;
> >  }
> >
> > -/**
> > - * drm_syncobj_add_callback - adds a callback to syncobj::cb_list
> > - * @syncobj: Sync object to which to add the callback
> > - * @cb: Callback to add
> > - * @func: Func to use when initializing the drm_syncobj_cb struct
> > - *
> > - * This adds a callback to be called next time the fence is replaced
> > - */
> >  void drm_syncobj_add_callback(struct drm_syncobj *syncobj,
> >                               struct drm_syncobj_cb *cb,
> >                               drm_syncobj_func_t func)
> > @@ -136,13 +128,7 @@ void drm_syncobj_add_callback(struct drm_syncobj
> > *syncobj,
> >         drm_syncobj_add_callback_locked(syncobj, cb, func);
> >         spin_unlock(&syncobj->lock);
> >  }
> > -EXPORT_SYMBOL(drm_syncobj_add_callback);
> >
> > -/**
> > - * drm_syncobj_add_callback - removes a callback to syncobj::cb_list
> > - * @syncobj: Sync object from which to remove the callback
> > - * @cb: Callback to remove
> > - */
> >  void drm_syncobj_remove_callback(struct drm_syncobj *syncobj,
> >                                  struct drm_syncobj_cb *cb)
> >  {
> > @@ -150,7 +136,6 @@ void drm_syncobj_remove_callback(struct drm_syncobj
> > *syncobj,
> >         list_del_init(&cb->node);
> >         spin_unlock(&syncobj->lock);
> >  }
> > -EXPORT_SYMBOL(drm_syncobj_remove_callback);
> >
> >  /**
> >   * drm_syncobj_replace_fence - replace fence in a sync object.
> > diff --git a/include/drm/drm_syncobj.h b/include/drm/drm_syncobj.h
> > index 3980602472c0..e419c79ba94d 100644
> > --- a/include/drm/drm_syncobj.h
> > +++ b/include/drm/drm_syncobj.h
> > @@ -131,11 +131,6 @@ drm_syncobj_fence_get(struct drm_syncobj *syncobj)
> >
> >  struct drm_syncobj *drm_syncobj_find(struct drm_file *file_private,
> >                                      u32 handle);
> > -void drm_syncobj_add_callback(struct drm_syncobj *syncobj,
> > -                             struct drm_syncobj_cb *cb,
> > -                             drm_syncobj_func_t func);
> > -void drm_syncobj_remove_callback(struct drm_syncobj *syncobj,
> > -                                struct drm_syncobj_cb *cb);
> >  void drm_syncobj_replace_fence(struct drm_syncobj *syncobj,
> >                                struct dma_fence *fence);
> >  int drm_syncobj_find_fence(struct drm_file *file_private,
> > --
> > 2.18.0
> >
> >

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx