Common pipe screen ref counting

Submitted by Rob Herring on June 17, 2016, 5:45 p.m.

Details

Reviewer None
Submitted June 17, 2016, 5:45 p.m.
Last Updated Aug. 7, 2017, 11 p.m.
Revision 5

Cover Letter(s)

Revision 1
      I needed to add screen ref counting to vc4 driver, so rather than yet 
another copy of the same fd hashing and ref counting code, I implemented 
it in the pipe-loader. AFAICT, the pipe-loader is the only place the 
winsys create screen functions are called and seemed to be the best 
location to put this. The tricky part is the destroy path and not 
freeing the screen before reference counting. I think I found all the 
callers of pipe_screen->destroy.

This is tested on virgl and freedreno on Android and radeon is build 
tested only.

Rob

Rob Herring (7):
  gallium: move pipe_screen destroy into pipe-loader
  pipe-loader-drm: Add common pipe_screen refcounting
  Revert "virgl: reuse screen when fd is already open"
  Revert "virgl: mark function as static"
  nouveau: remove screen ref counting
  freedreno: remove screen ref counting
  radeon: remove screen ref counting

 src/gallium/auxiliary/pipe-loader/pipe_loader.h    |  1 +
 .../auxiliary/pipe-loader/pipe_loader_drm.c        | 71 ++++++++++++++++-
 src/gallium/auxiliary/pipe-loader/pipe_loader_sw.c |  6 ++
 src/gallium/auxiliary/target-helpers/drm_helper.h  |  7 +-
 src/gallium/auxiliary/vl/vl_winsys_dri.c           |  1 -
 src/gallium/auxiliary/vl/vl_winsys_dri3.c          |  1 -
 src/gallium/auxiliary/vl/vl_winsys_drm.c           |  1 -
 src/gallium/drivers/freedreno/freedreno_screen.c   |  1 -
 src/gallium/drivers/freedreno/freedreno_screen.h   | 10 ---
 src/gallium/drivers/nouveau/nouveau_screen.h       |  2 -
 src/gallium/drivers/r300/r300_screen.c             |  3 -
 src/gallium/drivers/r600/r600_pipe.c               |  6 --
 src/gallium/drivers/radeon/radeon_winsys.h         |  8 --
 src/gallium/drivers/radeonsi/si_pipe.c             |  6 --
 src/gallium/drivers/virgl/virgl_screen.c           |  1 -
 src/gallium/drivers/virgl/virgl_screen.h           |  6 --
 src/gallium/include/pipe/p_screen.h                |  1 +
 src/gallium/state_trackers/clover/core/device.cpp  |  4 +-
 src/gallium/state_trackers/dri/dri_screen.c        |  3 -
 src/gallium/state_trackers/xa/xa_tracker.c         |  2 -
 src/gallium/tests/trivial/compute.c                |  1 -
 src/gallium/tests/trivial/quad-tex.c               |  1 -
 src/gallium/tests/trivial/tri.c                    |  1 -
 src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c      | 66 +---------------
 src/gallium/winsys/amdgpu/drm/amdgpu_winsys.h      |  1 -
 .../winsys/freedreno/drm/freedreno_drm_winsys.c    | 89 +---------------------
 .../winsys/nouveau/drm/nouveau_drm_winsys.c        | 84 +-------------------
 src/gallium/winsys/radeon/drm/radeon_drm_winsys.c  | 80 +------------------
 src/gallium/winsys/virgl/drm/virgl_drm_public.h    |  4 +-
 src/gallium/winsys/virgl/drm/virgl_drm_winsys.c    | 89 +---------------------
 30 files changed, 95 insertions(+), 462 deletions(-)
    
Revision 2
      I needed to add screen ref counting to vc4 driver, so rather than yet
another copy of the same fd hashing and ref counting code, I implemented
it in the pipe-loader. AFAICT, the pipe-loader is the only place the
winsys create screen functions are called and seemed to be the best
location to put this. The tricky part is the destroy path and not
freeing the screen before reference counting. I think I found all the
callers of pipe_screen->destroy.

This version differs from the RFC in that the fd hashing and ref 
counting are opt-in as library functions and winsys implementations can 
do something different as amdgpu does. This makes the ref counting a bit 
more complicated to understand in that it happens in different places.

I'm testing on virgl, freedreno, and vc4. Testing on the other platforms 
would be great.

Rob

Rob Herring (11):
  gallium: move pipe_screen destroy into pipe-loader
  gallium: add common pipe_screen reference counting functions
  pipe-loader-drm: protect create_screen() and destroy() calls with
    mutex
  pipe-loader-drm: use pipe_screen_unreference to destroy screen
  nouveau: use common screen ref counting
  freedreno: use common screen ref counting
  amdgpu: use common screen ref counting
  radeon: use common screen ref counting
  vmwgfx: use common screen ref counting
  vc4: use common screen ref counting
  virgl: use common screen ref counting

 src/gallium/auxiliary/Makefile.sources             |   2 +
 src/gallium/auxiliary/pipe-loader/pipe_loader.h    |   1 +
 .../auxiliary/pipe-loader/pipe_loader_drm.c        |  30 ++++--
 src/gallium/auxiliary/pipe-loader/pipe_loader_sw.c |   6 ++
 src/gallium/auxiliary/util/u_screen.c              | 104 +++++++++++++++++++++
 src/gallium/auxiliary/util/u_screen.h              |  32 +++++++
 src/gallium/auxiliary/vl/vl_winsys_dri.c           |   1 -
 src/gallium/auxiliary/vl/vl_winsys_dri3.c          |   1 -
 src/gallium/auxiliary/vl/vl_winsys_drm.c           |   1 -
 src/gallium/drivers/freedreno/freedreno_screen.c   |   1 -
 src/gallium/drivers/freedreno/freedreno_screen.h   |  10 --
 src/gallium/drivers/nouveau/nouveau_screen.c       |   6 --
 src/gallium/drivers/nouveau/nouveau_screen.h       |   4 -
 src/gallium/drivers/nouveau/nv30/nv30_screen.c     |   3 -
 src/gallium/drivers/nouveau/nv50/nv50_screen.c     |   3 -
 src/gallium/drivers/nouveau/nvc0/nvc0_screen.c     |   3 -
 src/gallium/drivers/r300/r300_screen.c             |   3 -
 src/gallium/drivers/r600/r600_pipe.c               |   6 --
 src/gallium/drivers/radeon/radeon_winsys.h         |   8 --
 src/gallium/drivers/radeonsi/si_pipe.c             |   6 --
 src/gallium/include/pipe/p_screen.h                |   2 +
 src/gallium/state_trackers/clover/core/device.cpp  |   4 +-
 src/gallium/state_trackers/dri/dri_screen.c        |   3 -
 src/gallium/state_trackers/xa/xa_tracker.c         |   2 -
 src/gallium/tests/trivial/compute.c                |   1 -
 src/gallium/tests/trivial/quad-tex.c               |   1 -
 src/gallium/tests/trivial/tri.c                    |   1 -
 src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c      |  45 ++-------
 src/gallium/winsys/amdgpu/drm/amdgpu_winsys.h      |   1 -
 .../winsys/freedreno/drm/freedreno_drm_winsys.c    |  94 ++-----------------
 .../winsys/nouveau/drm/nouveau_drm_winsys.c        |  89 ++----------------
 src/gallium/winsys/radeon/drm/radeon_drm_winsys.c  |  84 ++---------------
 src/gallium/winsys/svga/drm/vmw_screen.c           |  51 ++--------
 src/gallium/winsys/svga/drm/vmw_screen.h           |   6 --
 src/gallium/winsys/vc4/drm/vc4_drm_winsys.c        |   9 +-
 src/gallium/winsys/virgl/drm/virgl_drm_winsys.c    |  86 ++---------------
 36 files changed, 219 insertions(+), 491 deletions(-)
 create mode 100644 src/gallium/auxiliary/util/u_screen.c
 create mode 100644 src/gallium/auxiliary/util/u_screen.h
    
Revision 3
      Another version of common pipe_screen reference counting.

Changes in v3:
- dup() fd and store in pipe_screen as the lifetime of the 
pipe_loader_drm_device and pipe_screen are different.
- Fix leaking of pipe_loader_drm_device. Only the last one closed was 
getting freed.
- Move mutex for fd hash table into u_screen.c

Rob

Rob Herring (11):
  gallium: move pipe_screen destroy into pipe-loader
  pipe-loader-drm: protect create_screen() calls with a mutex
  gallium: add common pipe_screen reference counting functions
  pipe-loader-drm: use pipe_screen_unreference to destroy screen
  nouveau: use common screen ref counting
  freedreno: use common screen ref counting
  amdgpu: use common screen ref counting
  radeon: use common screen ref counting
  vmwgfx: use common screen ref counting
  vc4: use common screen ref counting
  virgl: use common screen ref counting

 src/gallium/auxiliary/Makefile.sources             |   2 +
 src/gallium/auxiliary/pipe-loader/pipe_loader.h    |   1 +
 .../auxiliary/pipe-loader/pipe_loader_drm.c        |  15 ++-
 src/gallium/auxiliary/pipe-loader/pipe_loader_sw.c |   6 ++
 src/gallium/auxiliary/util/u_screen.c              | 114 +++++++++++++++++++++
 src/gallium/auxiliary/util/u_screen.h              |  32 ++++++
 src/gallium/auxiliary/vl/vl_winsys_dri.c           |   1 -
 src/gallium/auxiliary/vl/vl_winsys_dri3.c          |   1 -
 src/gallium/auxiliary/vl/vl_winsys_drm.c           |   1 -
 src/gallium/drivers/freedreno/freedreno_screen.c   |   1 -
 src/gallium/drivers/freedreno/freedreno_screen.h   |  10 --
 src/gallium/drivers/nouveau/nouveau_screen.c       |   6 --
 src/gallium/drivers/nouveau/nouveau_screen.h       |   4 -
 src/gallium/drivers/nouveau/nv30/nv30_screen.c     |   3 -
 src/gallium/drivers/nouveau/nv50/nv50_screen.c     |   3 -
 src/gallium/drivers/nouveau/nvc0/nvc0_screen.c     |   3 -
 src/gallium/drivers/r300/r300_screen.c             |   3 -
 src/gallium/drivers/r600/r600_pipe.c               |   6 --
 src/gallium/drivers/radeon/radeon_winsys.h         |   8 --
 src/gallium/drivers/radeonsi/si_pipe.c             |   6 --
 src/gallium/include/pipe/p_screen.h                |   3 +
 src/gallium/state_trackers/clover/core/device.cpp  |   4 +-
 src/gallium/state_trackers/dri/dri_screen.c        |   3 -
 src/gallium/state_trackers/xa/xa_tracker.c         |   2 -
 src/gallium/tests/trivial/compute.c                |   1 -
 src/gallium/tests/trivial/quad-tex.c               |   1 -
 src/gallium/tests/trivial/tri.c                    |   1 -
 src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c      |  45 ++------
 src/gallium/winsys/amdgpu/drm/amdgpu_winsys.h      |   1 -
 .../winsys/freedreno/drm/freedreno_drm_winsys.c    |  94 ++---------------
 .../winsys/nouveau/drm/nouveau_drm_winsys.c        |  89 ++--------------
 src/gallium/winsys/radeon/drm/radeon_drm_winsys.c  |  84 ++-------------
 src/gallium/winsys/svga/drm/vmw_screen.c           |  51 ++-------
 src/gallium/winsys/svga/drm/vmw_screen.h           |   6 --
 src/gallium/winsys/vc4/drm/vc4_drm_winsys.c        |   9 +-
 src/gallium/winsys/virgl/drm/virgl_drm_winsys.c    |  86 ++--------------
 36 files changed, 221 insertions(+), 485 deletions(-)
 create mode 100644 src/gallium/auxiliary/util/u_screen.c
 create mode 100644 src/gallium/auxiliary/util/u_screen.h
    
Revision 4
      Another version of common pipe_screen reference counting. Please help 
test on AMD and Nouveau as those are the more complicated ones and I 
don't have h/w to test.

Changes in v4:
- Move fd dup() back into driver winsys create screen functions which 
  sometimes need the dup'ed fd before the pipe_screen is created.
- Update vmwgfx driver which I missed updating in v3
- Update vc4 commit msg to reflect this is a new feature.

Changes in v3:
- dup() fd and store in pipe_screen as the lifetime of the
pipe_loader_drm_device and pipe_screen are different.
- Fix leaking of pipe_loader_drm_device. Only the last one closed was
getting freed.
- Move mutex for fd hash table into u_screen.c

Rob


Rob Herring (11):
  gallium: move pipe_screen destroy into pipe-loader
  pipe-loader-drm: protect create_screen() calls with a mutex
  gallium: add common pipe_screen reference counting functions
  pipe-loader-drm: use pipe_screen_unreference to destroy screen
  nouveau: use common screen ref counting
  freedreno: use common screen ref counting
  amdgpu: use common screen ref counting
  radeon: use common screen ref counting
  vmwgfx: use common screen ref counting
  virgl: use common screen ref counting
  vc4: use common screen ref counting

 src/gallium/auxiliary/Makefile.sources             |   2 +
 src/gallium/auxiliary/pipe-loader/pipe_loader.h    |   1 +
 .../auxiliary/pipe-loader/pipe_loader_drm.c        |  15 ++-
 src/gallium/auxiliary/pipe-loader/pipe_loader_sw.c |   6 ++
 src/gallium/auxiliary/target-helpers/drm_helper.h  |   2 +-
 src/gallium/auxiliary/util/u_screen.c              | 114 +++++++++++++++++++++
 src/gallium/auxiliary/util/u_screen.h              |  32 ++++++
 src/gallium/auxiliary/vl/vl_winsys_dri.c           |   1 -
 src/gallium/auxiliary/vl/vl_winsys_dri3.c          |   1 -
 src/gallium/auxiliary/vl/vl_winsys_drm.c           |   1 -
 src/gallium/drivers/freedreno/freedreno_screen.c   |   1 -
 src/gallium/drivers/freedreno/freedreno_screen.h   |  10 --
 src/gallium/drivers/nouveau/nouveau_screen.c       |   6 --
 src/gallium/drivers/nouveau/nouveau_screen.h       |   4 -
 src/gallium/drivers/nouveau/nv30/nv30_screen.c     |   3 -
 src/gallium/drivers/nouveau/nv50/nv50_screen.c     |   3 -
 src/gallium/drivers/nouveau/nvc0/nvc0_screen.c     |   3 -
 src/gallium/drivers/r300/r300_screen.c             |   3 -
 src/gallium/drivers/r600/r600_pipe.c               |   6 --
 src/gallium/drivers/radeon/radeon_winsys.h         |   8 --
 src/gallium/drivers/radeonsi/si_pipe.c             |   6 --
 src/gallium/drivers/svga/svga_public.h             |   2 +-
 src/gallium/drivers/svga/svga_screen.c             |   5 +-
 src/gallium/include/pipe/p_screen.h                |   3 +
 src/gallium/state_trackers/clover/core/device.cpp  |   4 +-
 src/gallium/state_trackers/dri/dri_screen.c        |   3 -
 src/gallium/state_trackers/xa/xa_tracker.c         |   2 -
 src/gallium/targets/pipe-loader/pipe_vmwgfx.c      |   2 +-
 src/gallium/tests/trivial/compute.c                |   1 -
 src/gallium/tests/trivial/quad-tex.c               |   1 -
 src/gallium/tests/trivial/tri.c                    |   1 -
 src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c      |  45 ++------
 src/gallium/winsys/amdgpu/drm/amdgpu_winsys.h      |   1 -
 .../winsys/freedreno/drm/freedreno_drm_winsys.c    |  98 ++----------------
 .../winsys/nouveau/drm/nouveau_drm_winsys.c        |  69 +------------
 src/gallium/winsys/radeon/drm/radeon_drm_winsys.c  |  80 ++-------------
 src/gallium/winsys/svga/drm/vmw_screen.c           |  54 ++--------
 src/gallium/winsys/svga/drm/vmw_screen.h           |   6 --
 src/gallium/winsys/vc4/drm/vc4_drm_winsys.c        |  11 +-
 src/gallium/winsys/virgl/drm/virgl_drm_winsys.c    |  88 ++--------------
 40 files changed, 236 insertions(+), 468 deletions(-)
 create mode 100644 src/gallium/auxiliary/util/u_screen.c
 create mode 100644 src/gallium/auxiliary/util/u_screen.h
    
Revision 5
      I finally got around to updating this series since I have another driver 
to add reference counting to (kms_swrast). Reference counting of the pipe
screen is necessary for Android support (and VDPAU AIUI), but each driver
has so far implemented there own private ref counting. This series creates
a common implementation. The previous version is here[1].

Please help test! I've tested with virgl and freedreno on Android and 
don't have other h/w. A branch with this series is available here[2].

Changes in v5:
- Rebased to current master.
- Moved the pipe loader changes from pipe_loader_drm.c to pipe_loader.c 
  to also support software rendering.
- Export pipe_loader_create_screen and pipe_loader_release.
- Removed the fd hash table mutex. It is not needed because the loader 
  mutex is sufficient.
- Combined radeon and amdgpu patch due to common dependence on 
  radeon_winsys.h and amdgpu fallback path.
- Fixed bug in radeon_drm_winsys_create returning the pipe_screen instead
  of the struct radeon_winsys.
- Support the case when the pipe_screen->fd is -1 (AMDGPU).
- Reworked SVGA driver to avoid dup/fcntl in svga_screen.c as that's not 
  available on Windows.
- Fixed support of debug wrappers 
- Added etnaviv support

Rob

[1] http://comments.gmane.org/gmane.comp.video.mesa3d.devel/142722
[2] https://github.com/robherring/mesa.git screen-refcnt-v5

Rob Herring (12):
  gallium: move pipe_screen destroy into pipe-loader
  pipe-loader: serialize create_screen() calls with a mutex
  gallium: add common pipe_screen reference counting functions
  gallium: use pipe_screen_unreference to destroy screen in debug
    wrappers
  pipe-loader: use pipe_screen_unreference to destroy screen
  etnaviv: use common pipe_screen ref counting
  freedreno: use common pipe_screen ref counting
  nouveau: use common pipe_screen ref counting
  radeon: use common pipe_screen ref counting
  vmwgfx: use common pipe_screen ref counting
  virgl: use common pipe_screen ref counting
  vc4: add pipe_screen ref counting

 src/gallium/auxiliary/Makefile.sources             |   2 +
 src/gallium/auxiliary/pipe-loader/pipe_loader.c    |  28 +++++-
 src/gallium/auxiliary/pipe-loader/pipe_loader.h    |   1 +
 src/gallium/auxiliary/target-helpers/drm_helper.h  |  21 ++--
 src/gallium/auxiliary/util/u_screen.c              | 112 +++++++++++++++++++++
 src/gallium/auxiliary/util/u_screen.h              |  32 ++++++
 src/gallium/auxiliary/vl/vl_winsys_dri.c           |   1 -
 src/gallium/auxiliary/vl/vl_winsys_dri3.c          |   1 -
 src/gallium/auxiliary/vl/vl_winsys_drm.c           |   1 -
 src/gallium/drivers/ddebug/dd_screen.c             |   3 +-
 src/gallium/drivers/etnaviv/etnaviv_screen.c       |   1 -
 src/gallium/drivers/etnaviv/etnaviv_screen.h       |   4 -
 src/gallium/drivers/freedreno/freedreno_screen.c   |   1 -
 src/gallium/drivers/freedreno/freedreno_screen.h   |  10 --
 src/gallium/drivers/noop/noop_pipe.c               |   3 +-
 src/gallium/drivers/nouveau/nouveau_screen.c       |   6 --
 src/gallium/drivers/nouveau/nouveau_screen.h       |   4 -
 src/gallium/drivers/nouveau/nv30/nv30_screen.c     |   3 -
 src/gallium/drivers/nouveau/nv50/nv50_screen.c     |   3 -
 src/gallium/drivers/nouveau/nvc0/nvc0_screen.c     |   3 -
 src/gallium/drivers/r300/r300_screen.c             |   3 -
 src/gallium/drivers/r600/r600_pipe.c               |   6 --
 src/gallium/drivers/radeon/radeon_winsys.h         |   8 --
 src/gallium/drivers/radeonsi/si_pipe.c             |   3 -
 src/gallium/drivers/rbug/rbug_screen.c             |   3 +-
 src/gallium/drivers/trace/tr_screen.c              |   3 +-
 src/gallium/include/pipe/p_screen.h                |   5 +
 src/gallium/state_trackers/clover/core/device.cpp  |   4 +-
 src/gallium/state_trackers/dri/dri_screen.c        |   3 -
 src/gallium/state_trackers/xa/xa_tracker.c         |   2 -
 src/gallium/targets/dri-vdpau.dyn                  |   2 +
 src/gallium/targets/dri/dri.sym                    |   2 +
 src/gallium/targets/pipe-loader/pipe_vmwgfx.c      |  24 +++--
 src/gallium/targets/vdpau/vdpau.sym                |   2 +
 src/gallium/tests/trivial/compute.c                |   1 -
 src/gallium/tests/trivial/quad-tex.c               |   1 -
 src/gallium/tests/trivial/tri.c                    |   1 -
 src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c      |  44 +-------
 src/gallium/winsys/amdgpu/drm/amdgpu_winsys.h      |   1 -
 .../winsys/etnaviv/drm/etnaviv_drm_winsys.c        |  81 ++-------------
 .../winsys/freedreno/drm/freedreno_drm_winsys.c    |  98 ++----------------
 .../winsys/nouveau/drm/nouveau_drm_winsys.c        |  69 +------------
 src/gallium/winsys/radeon/drm/radeon_drm_winsys.c  |  79 ++-------------
 src/gallium/winsys/radeon/drm/radeon_drm_winsys.h  |   1 -
 src/gallium/winsys/svga/drm/vmw_screen.c           |  55 ++--------
 src/gallium/winsys/svga/drm/vmw_screen.h           |   6 --
 src/gallium/winsys/vc4/drm/vc4_drm_winsys.c        |  21 +++-
 src/gallium/winsys/virgl/drm/virgl_drm_winsys.c    |  88 ++--------------
 48 files changed, 290 insertions(+), 566 deletions(-)
 create mode 100644 src/gallium/auxiliary/util/u_screen.c
 create mode 100644 src/gallium/auxiliary/util/u_screen.h
    

Revisions