Discussion:
[PATCH v5 00/12] Common pipe screen ref counting
Add Reply
Rob Herring
2017-08-07 22:57:59 UTC
Reply
Permalink
Raw Message
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
--
2.11.0
Rob Herring
2017-08-07 22:58:00 UTC
Reply
Permalink
Raw Message
In preparation to add reference counting of pipe_screen in the pipe-loader,
pipe_loader_release needs to destroy the pipe_screen instead of state
trackers.

Signed-off-by: Rob Herring <***@kernel.org>
Cc: Emil Velikov <***@gmail.com>
---
src/gallium/auxiliary/pipe-loader/pipe_loader.c | 14 ++++++++++++--
src/gallium/auxiliary/pipe-loader/pipe_loader.h | 1 +
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/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 -
11 files changed, 14 insertions(+), 16 deletions(-)

diff --git a/src/gallium/auxiliary/pipe-loader/pipe_loader.c b/src/gallium/auxiliary/pipe-loader/pipe_loader.c
index 926db49fd24b..61e5786a2528 100644
--- a/src/gallium/auxiliary/pipe-loader/pipe_loader.c
+++ b/src/gallium/auxiliary/pipe-loader/pipe_loader.c
@@ -67,13 +67,20 @@ pipe_loader_probe(struct pipe_loader_device **devs, int ndev)
return n;
}

+static void
+pipe_loader_release_dev(struct pipe_loader_device *dev)
+{
+ dev->pscreen->destroy(dev->pscreen);
+ dev->ops->release(&dev);
+}
+
void
pipe_loader_release(struct pipe_loader_device **devs, int ndev)
{
int i;

for (i = 0; i < ndev; i++)
- devs[i]->ops->release(&devs[i]);
+ pipe_loader_release_dev(devs[i]);
}

void
@@ -125,12 +132,15 @@ pipe_loader_get_driinfo_xml(const char *driver_name)
struct pipe_screen *
pipe_loader_create_screen(struct pipe_loader_device *dev)
{
+ struct pipe_screen *pscreen;
struct pipe_screen_config config;

pipe_loader_load_options(dev);
config.options = &dev->option_cache;

- return dev->ops->create_screen(dev, &config);
+ pscreen = dev->ops->create_screen(dev, &config);
+ dev->pscreen = pscreen;
+ return pscreen;
}

struct util_dl_library *
diff --git a/src/gallium/auxiliary/pipe-loader/pipe_loader.h b/src/gallium/auxiliary/pipe-loader/pipe_loader.h
index b50114310b4a..25cf5616f785 100644
--- a/src/gallium/auxiliary/pipe-loader/pipe_loader.h
+++ b/src/gallium/auxiliary/pipe-loader/pipe_loader.h
@@ -66,6 +66,7 @@ struct pipe_loader_device {

char *driver_name;
const struct pipe_loader_ops *ops;
+ struct pipe_screen *pscreen;

driOptionCache option_cache;
driOptionCache option_info;
diff --git a/src/gallium/auxiliary/vl/vl_winsys_dri.c b/src/gallium/auxiliary/vl/vl_winsys_dri.c
index b4fb47ea8e46..444fff321eae 100644
--- a/src/gallium/auxiliary/vl/vl_winsys_dri.c
+++ b/src/gallium/auxiliary/vl/vl_winsys_dri.c
@@ -463,7 +463,6 @@ vl_dri2_screen_destroy(struct vl_screen *vscreen)
}

vl_dri2_destroy_drawable(scrn);
- scrn->base.pscreen->destroy(scrn->base.pscreen);
pipe_loader_release(&scrn->base.dev, 1);
FREE(scrn);
}
diff --git a/src/gallium/auxiliary/vl/vl_winsys_dri3.c b/src/gallium/auxiliary/vl/vl_winsys_dri3.c
index 8251087f3f90..4ed7ef0eacad 100644
--- a/src/gallium/auxiliary/vl/vl_winsys_dri3.c
+++ b/src/gallium/auxiliary/vl/vl_winsys_dri3.c
@@ -732,7 +732,6 @@ vl_dri3_screen_destroy(struct vl_screen *vscreen)
xcb_unregister_for_special_event(scrn->conn, scrn->special_event);
}
scrn->pipe->destroy(scrn->pipe);
- scrn->base.pscreen->destroy(scrn->base.pscreen);
pipe_loader_release(&scrn->base.dev, 1);
FREE(scrn);

diff --git a/src/gallium/auxiliary/vl/vl_winsys_drm.c b/src/gallium/auxiliary/vl/vl_winsys_drm.c
index df8809c501cb..6bbc87635c78 100644
--- a/src/gallium/auxiliary/vl/vl_winsys_drm.c
+++ b/src/gallium/auxiliary/vl/vl_winsys_drm.c
@@ -81,7 +81,6 @@ vl_drm_screen_destroy(struct vl_screen *vscreen)
{
assert(vscreen);

- vscreen->pscreen->destroy(vscreen->pscreen);
pipe_loader_release(&vscreen->dev, 1);
FREE(vscreen);
}
diff --git a/src/gallium/state_trackers/clover/core/device.cpp b/src/gallium/state_trackers/clover/core/device.cpp
index 8dfba1ad9fd9..bfdd32c794a1 100644
--- a/src/gallium/state_trackers/clover/core/device.cpp
+++ b/src/gallium/state_trackers/clover/core/device.cpp
@@ -45,14 +45,12 @@ device::device(clover::platform &platform, pipe_loader_device *ldev) :
pipe = pipe_loader_create_screen(ldev);
if (!pipe || !pipe->get_param(pipe, PIPE_CAP_COMPUTE)) {
if (pipe)
- pipe->destroy(pipe);
+ pipe_loader_release(&ldev, 1);
throw error(CL_INVALID_DEVICE);
}
}

device::~device() {
- if (pipe)
- pipe->destroy(pipe);
if (ldev)
pipe_loader_release(&ldev, 1);
}
diff --git a/src/gallium/state_trackers/dri/dri_screen.c b/src/gallium/state_trackers/dri/dri_screen.c
index 406e97dc2429..01ca2202b4c8 100644
--- a/src/gallium/state_trackers/dri/dri_screen.c
+++ b/src/gallium/state_trackers/dri/dri_screen.c
@@ -428,9 +428,6 @@ dri_destroy_screen_helper(struct dri_screen * screen)
if (screen->st_api && screen->st_api->destroy)
screen->st_api->destroy(screen->st_api);

- if (screen->base.screen)
- screen->base.screen->destroy(screen->base.screen);
-
mtx_destroy(&screen->opencl_func_mutex);
}

diff --git a/src/gallium/state_trackers/xa/xa_tracker.c b/src/gallium/state_trackers/xa/xa_tracker.c
index 03a3abf6835a..fee83afcc66e 100644
--- a/src/gallium/state_trackers/xa/xa_tracker.c
+++ b/src/gallium/state_trackers/xa/xa_tracker.c
@@ -209,7 +209,6 @@ xa_tracker_create(int drm_fd)
out_sf_alloc_fail:
xa_context_destroy(xa->default_ctx);
out_no_pipe:
- xa->screen->destroy(xa->screen);
out_no_screen:
if (xa->dev)
pipe_loader_release(&xa->dev, 1);
@@ -225,7 +224,6 @@ xa_tracker_destroy(struct xa_tracker *xa)
{
free(xa->supported_formats);
xa_context_destroy(xa->default_ctx);
- xa->screen->destroy(xa->screen);
pipe_loader_release(&xa->dev, 1);
free(xa);
}
diff --git a/src/gallium/tests/trivial/compute.c b/src/gallium/tests/trivial/compute.c
index 443451e13d24..d718906eb7c8 100644
--- a/src/gallium/tests/trivial/compute.c
+++ b/src/gallium/tests/trivial/compute.c
@@ -90,7 +90,6 @@ static void init_ctx(struct context *ctx)
static void destroy_ctx(struct context *ctx)
{
ctx->pipe->destroy(ctx->pipe);
- ctx->screen->destroy(ctx->screen);
pipe_loader_release(&ctx->dev, 1);
FREE(ctx);
}
diff --git a/src/gallium/tests/trivial/quad-tex.c b/src/gallium/tests/trivial/quad-tex.c
index 2ee544a41294..8b8352d5c535 100644
--- a/src/gallium/tests/trivial/quad-tex.c
+++ b/src/gallium/tests/trivial/quad-tex.c
@@ -291,7 +291,6 @@ static void close_prog(struct program *p)
pipe_resource_reference(&p->vbuf, NULL);

p->pipe->destroy(p->pipe);
- p->screen->destroy(p->screen);
pipe_loader_release(&p->dev, 1);

FREE(p);
diff --git a/src/gallium/tests/trivial/tri.c b/src/gallium/tests/trivial/tri.c
index a2031696f029..bb053e761b75 100644
--- a/src/gallium/tests/trivial/tri.c
+++ b/src/gallium/tests/trivial/tri.c
@@ -231,7 +231,6 @@ static void close_prog(struct program *p)
pipe_resource_reference(&p->vbuf, NULL);

p->pipe->destroy(p->pipe);
- p->screen->destroy(p->screen);
pipe_loader_release(&p->dev, 1);

FREE(p);
--
2.11.0
Nicolai Hähnle
2017-08-09 16:47:28 UTC
Reply
Permalink
Raw Message
Post by Rob Herring
In preparation to add reference counting of pipe_screen in the pipe-loader,
pipe_loader_release needs to destroy the pipe_screen instead of state
trackers.
Did you miss Nine?

Cheers,
Nicolai
Post by Rob Herring
---
src/gallium/auxiliary/pipe-loader/pipe_loader.c | 14 ++++++++++++--
src/gallium/auxiliary/pipe-loader/pipe_loader.h | 1 +
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/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 -
11 files changed, 14 insertions(+), 16 deletions(-)
diff --git a/src/gallium/auxiliary/pipe-loader/pipe_loader.c b/src/gallium/auxiliary/pipe-loader/pipe_loader.c
index 926db49fd24b..61e5786a2528 100644
--- a/src/gallium/auxiliary/pipe-loader/pipe_loader.c
+++ b/src/gallium/auxiliary/pipe-loader/pipe_loader.c
@@ -67,13 +67,20 @@ pipe_loader_probe(struct pipe_loader_device **devs, int ndev)
return n;
}
+static void
+pipe_loader_release_dev(struct pipe_loader_device *dev)
+{
+ dev->pscreen->destroy(dev->pscreen);
+ dev->ops->release(&dev);
+}
+
void
pipe_loader_release(struct pipe_loader_device **devs, int ndev)
{
int i;
for (i = 0; i < ndev; i++)
- devs[i]->ops->release(&devs[i]);
+ pipe_loader_release_dev(devs[i]);
}
void
@@ -125,12 +132,15 @@ pipe_loader_get_driinfo_xml(const char *driver_name)
struct pipe_screen *
pipe_loader_create_screen(struct pipe_loader_device *dev)
{
+ struct pipe_screen *pscreen;
struct pipe_screen_config config;
pipe_loader_load_options(dev);
config.options = &dev->option_cache;
- return dev->ops->create_screen(dev, &config);
+ pscreen = dev->ops->create_screen(dev, &config);
+ dev->pscreen = pscreen;
+ return pscreen;
}
struct util_dl_library *
diff --git a/src/gallium/auxiliary/pipe-loader/pipe_loader.h b/src/gallium/auxiliary/pipe-loader/pipe_loader.h
index b50114310b4a..25cf5616f785 100644
--- a/src/gallium/auxiliary/pipe-loader/pipe_loader.h
+++ b/src/gallium/auxiliary/pipe-loader/pipe_loader.h
@@ -66,6 +66,7 @@ struct pipe_loader_device {
char *driver_name;
const struct pipe_loader_ops *ops;
+ struct pipe_screen *pscreen;
driOptionCache option_cache;
driOptionCache option_info;
diff --git a/src/gallium/auxiliary/vl/vl_winsys_dri.c b/src/gallium/auxiliary/vl/vl_winsys_dri.c
index b4fb47ea8e46..444fff321eae 100644
--- a/src/gallium/auxiliary/vl/vl_winsys_dri.c
+++ b/src/gallium/auxiliary/vl/vl_winsys_dri.c
@@ -463,7 +463,6 @@ vl_dri2_screen_destroy(struct vl_screen *vscreen)
}
vl_dri2_destroy_drawable(scrn);
- scrn->base.pscreen->destroy(scrn->base.pscreen);
pipe_loader_release(&scrn->base.dev, 1);
FREE(scrn);
}
diff --git a/src/gallium/auxiliary/vl/vl_winsys_dri3.c b/src/gallium/auxiliary/vl/vl_winsys_dri3.c
index 8251087f3f90..4ed7ef0eacad 100644
--- a/src/gallium/auxiliary/vl/vl_winsys_dri3.c
+++ b/src/gallium/auxiliary/vl/vl_winsys_dri3.c
@@ -732,7 +732,6 @@ vl_dri3_screen_destroy(struct vl_screen *vscreen)
xcb_unregister_for_special_event(scrn->conn, scrn->special_event);
}
scrn->pipe->destroy(scrn->pipe);
- scrn->base.pscreen->destroy(scrn->base.pscreen);
pipe_loader_release(&scrn->base.dev, 1);
FREE(scrn);
diff --git a/src/gallium/auxiliary/vl/vl_winsys_drm.c b/src/gallium/auxiliary/vl/vl_winsys_drm.c
index df8809c501cb..6bbc87635c78 100644
--- a/src/gallium/auxiliary/vl/vl_winsys_drm.c
+++ b/src/gallium/auxiliary/vl/vl_winsys_drm.c
@@ -81,7 +81,6 @@ vl_drm_screen_destroy(struct vl_screen *vscreen)
{
assert(vscreen);
- vscreen->pscreen->destroy(vscreen->pscreen);
pipe_loader_release(&vscreen->dev, 1);
FREE(vscreen);
}
diff --git a/src/gallium/state_trackers/clover/core/device.cpp b/src/gallium/state_trackers/clover/core/device.cpp
index 8dfba1ad9fd9..bfdd32c794a1 100644
--- a/src/gallium/state_trackers/clover/core/device.cpp
+++ b/src/gallium/state_trackers/clover/core/device.cpp
pipe = pipe_loader_create_screen(ldev);
if (!pipe || !pipe->get_param(pipe, PIPE_CAP_COMPUTE)) {
if (pipe)
- pipe->destroy(pipe);
+ pipe_loader_release(&ldev, 1);
throw error(CL_INVALID_DEVICE);
}
}
device::~device() {
- if (pipe)
- pipe->destroy(pipe);
if (ldev)
pipe_loader_release(&ldev, 1);
}
diff --git a/src/gallium/state_trackers/dri/dri_screen.c b/src/gallium/state_trackers/dri/dri_screen.c
index 406e97dc2429..01ca2202b4c8 100644
--- a/src/gallium/state_trackers/dri/dri_screen.c
+++ b/src/gallium/state_trackers/dri/dri_screen.c
@@ -428,9 +428,6 @@ dri_destroy_screen_helper(struct dri_screen * screen)
if (screen->st_api && screen->st_api->destroy)
screen->st_api->destroy(screen->st_api);
- if (screen->base.screen)
- screen->base.screen->destroy(screen->base.screen);
-
mtx_destroy(&screen->opencl_func_mutex);
}
diff --git a/src/gallium/state_trackers/xa/xa_tracker.c b/src/gallium/state_trackers/xa/xa_tracker.c
index 03a3abf6835a..fee83afcc66e 100644
--- a/src/gallium/state_trackers/xa/xa_tracker.c
+++ b/src/gallium/state_trackers/xa/xa_tracker.c
@@ -209,7 +209,6 @@ xa_tracker_create(int drm_fd)
xa_context_destroy(xa->default_ctx);
- xa->screen->destroy(xa->screen);
if (xa->dev)
pipe_loader_release(&xa->dev, 1);
@@ -225,7 +224,6 @@ xa_tracker_destroy(struct xa_tracker *xa)
{
free(xa->supported_formats);
xa_context_destroy(xa->default_ctx);
- xa->screen->destroy(xa->screen);
pipe_loader_release(&xa->dev, 1);
free(xa);
}
diff --git a/src/gallium/tests/trivial/compute.c b/src/gallium/tests/trivial/compute.c
index 443451e13d24..d718906eb7c8 100644
--- a/src/gallium/tests/trivial/compute.c
+++ b/src/gallium/tests/trivial/compute.c
@@ -90,7 +90,6 @@ static void init_ctx(struct context *ctx)
static void destroy_ctx(struct context *ctx)
{
ctx->pipe->destroy(ctx->pipe);
- ctx->screen->destroy(ctx->screen);
pipe_loader_release(&ctx->dev, 1);
FREE(ctx);
}
diff --git a/src/gallium/tests/trivial/quad-tex.c b/src/gallium/tests/trivial/quad-tex.c
index 2ee544a41294..8b8352d5c535 100644
--- a/src/gallium/tests/trivial/quad-tex.c
+++ b/src/gallium/tests/trivial/quad-tex.c
@@ -291,7 +291,6 @@ static void close_prog(struct program *p)
pipe_resource_reference(&p->vbuf, NULL);
p->pipe->destroy(p->pipe);
- p->screen->destroy(p->screen);
pipe_loader_release(&p->dev, 1);
FREE(p);
diff --git a/src/gallium/tests/trivial/tri.c b/src/gallium/tests/trivial/tri.c
index a2031696f029..bb053e761b75 100644
--- a/src/gallium/tests/trivial/tri.c
+++ b/src/gallium/tests/trivial/tri.c
@@ -231,7 +231,6 @@ static void close_prog(struct program *p)
pipe_resource_reference(&p->vbuf, NULL);
p->pipe->destroy(p->pipe);
- p->screen->destroy(p->screen);
pipe_loader_release(&p->dev, 1);
FREE(p);
--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
Marek Olšák
2017-08-09 18:32:59 UTC
Reply
Permalink
Raw Message
With Nicolai's comment addressed, patches 1-2 are:

Reviewed-by: Marek Olšák <***@amd.com>

Marek
Post by Nicolai Hähnle
Post by Rob Herring
In preparation to add reference counting of pipe_screen in the pipe-loader,
pipe_loader_release needs to destroy the pipe_screen instead of state
trackers.
Did you miss Nine?
Cheers,
Nicolai
Post by Rob Herring
---
src/gallium/auxiliary/pipe-loader/pipe_loader.c | 14 ++++++++++++--
src/gallium/auxiliary/pipe-loader/pipe_loader.h | 1 +
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/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 -
11 files changed, 14 insertions(+), 16 deletions(-)
diff --git a/src/gallium/auxiliary/pipe-loader/pipe_loader.c
b/src/gallium/auxiliary/pipe-loader/pipe_loader.c
index 926db49fd24b..61e5786a2528 100644
--- a/src/gallium/auxiliary/pipe-loader/pipe_loader.c
+++ b/src/gallium/auxiliary/pipe-loader/pipe_loader.c
@@ -67,13 +67,20 @@ pipe_loader_probe(struct pipe_loader_device **devs, int ndev)
return n;
}
+static void
+pipe_loader_release_dev(struct pipe_loader_device *dev)
+{
+ dev->pscreen->destroy(dev->pscreen);
+ dev->ops->release(&dev);
+}
+
void
pipe_loader_release(struct pipe_loader_device **devs, int ndev)
{
int i;
for (i = 0; i < ndev; i++)
- devs[i]->ops->release(&devs[i]);
+ pipe_loader_release_dev(devs[i]);
}
void
@@ -125,12 +132,15 @@ pipe_loader_get_driinfo_xml(const char *driver_name)
struct pipe_screen *
pipe_loader_create_screen(struct pipe_loader_device *dev)
{
+ struct pipe_screen *pscreen;
struct pipe_screen_config config;
pipe_loader_load_options(dev);
config.options = &dev->option_cache;
- return dev->ops->create_screen(dev, &config);
+ pscreen = dev->ops->create_screen(dev, &config);
+ dev->pscreen = pscreen;
+ return pscreen;
}
struct util_dl_library *
diff --git a/src/gallium/auxiliary/pipe-loader/pipe_loader.h
b/src/gallium/auxiliary/pipe-loader/pipe_loader.h
index b50114310b4a..25cf5616f785 100644
--- a/src/gallium/auxiliary/pipe-loader/pipe_loader.h
+++ b/src/gallium/auxiliary/pipe-loader/pipe_loader.h
@@ -66,6 +66,7 @@ struct pipe_loader_device {
char *driver_name;
const struct pipe_loader_ops *ops;
+ struct pipe_screen *pscreen;
driOptionCache option_cache;
driOptionCache option_info;
diff --git a/src/gallium/auxiliary/vl/vl_winsys_dri.c
b/src/gallium/auxiliary/vl/vl_winsys_dri.c
index b4fb47ea8e46..444fff321eae 100644
--- a/src/gallium/auxiliary/vl/vl_winsys_dri.c
+++ b/src/gallium/auxiliary/vl/vl_winsys_dri.c
@@ -463,7 +463,6 @@ vl_dri2_screen_destroy(struct vl_screen *vscreen)
}
vl_dri2_destroy_drawable(scrn);
- scrn->base.pscreen->destroy(scrn->base.pscreen);
pipe_loader_release(&scrn->base.dev, 1);
FREE(scrn);
}
diff --git a/src/gallium/auxiliary/vl/vl_winsys_dri3.c
b/src/gallium/auxiliary/vl/vl_winsys_dri3.c
index 8251087f3f90..4ed7ef0eacad 100644
--- a/src/gallium/auxiliary/vl/vl_winsys_dri3.c
+++ b/src/gallium/auxiliary/vl/vl_winsys_dri3.c
@@ -732,7 +732,6 @@ vl_dri3_screen_destroy(struct vl_screen *vscreen)
xcb_unregister_for_special_event(scrn->conn, scrn->special_event);
}
scrn->pipe->destroy(scrn->pipe);
- scrn->base.pscreen->destroy(scrn->base.pscreen);
pipe_loader_release(&scrn->base.dev, 1);
FREE(scrn);
diff --git a/src/gallium/auxiliary/vl/vl_winsys_drm.c
b/src/gallium/auxiliary/vl/vl_winsys_drm.c
index df8809c501cb..6bbc87635c78 100644
--- a/src/gallium/auxiliary/vl/vl_winsys_drm.c
+++ b/src/gallium/auxiliary/vl/vl_winsys_drm.c
@@ -81,7 +81,6 @@ vl_drm_screen_destroy(struct vl_screen *vscreen)
{
assert(vscreen);
- vscreen->pscreen->destroy(vscreen->pscreen);
pipe_loader_release(&vscreen->dev, 1);
FREE(vscreen);
}
diff --git a/src/gallium/state_trackers/clover/core/device.cpp
b/src/gallium/state_trackers/clover/core/device.cpp
index 8dfba1ad9fd9..bfdd32c794a1 100644
--- a/src/gallium/state_trackers/clover/core/device.cpp
+++ b/src/gallium/state_trackers/clover/core/device.cpp
@@ -45,14 +45,12 @@ device::device(clover::platform &platform,
pipe = pipe_loader_create_screen(ldev);
if (!pipe || !pipe->get_param(pipe, PIPE_CAP_COMPUTE)) {
if (pipe)
- pipe->destroy(pipe);
+ pipe_loader_release(&ldev, 1);
throw error(CL_INVALID_DEVICE);
}
}
device::~device() {
- if (pipe)
- pipe->destroy(pipe);
if (ldev)
pipe_loader_release(&ldev, 1);
}
diff --git a/src/gallium/state_trackers/dri/dri_screen.c
b/src/gallium/state_trackers/dri/dri_screen.c
index 406e97dc2429..01ca2202b4c8 100644
--- a/src/gallium/state_trackers/dri/dri_screen.c
+++ b/src/gallium/state_trackers/dri/dri_screen.c
@@ -428,9 +428,6 @@ dri_destroy_screen_helper(struct dri_screen * screen)
if (screen->st_api && screen->st_api->destroy)
screen->st_api->destroy(screen->st_api);
- if (screen->base.screen)
- screen->base.screen->destroy(screen->base.screen);
-
mtx_destroy(&screen->opencl_func_mutex);
}
diff --git a/src/gallium/state_trackers/xa/xa_tracker.c
b/src/gallium/state_trackers/xa/xa_tracker.c
index 03a3abf6835a..fee83afcc66e 100644
--- a/src/gallium/state_trackers/xa/xa_tracker.c
+++ b/src/gallium/state_trackers/xa/xa_tracker.c
@@ -209,7 +209,6 @@ xa_tracker_create(int drm_fd)
xa_context_destroy(xa->default_ctx);
- xa->screen->destroy(xa->screen);
if (xa->dev)
pipe_loader_release(&xa->dev, 1);
@@ -225,7 +224,6 @@ xa_tracker_destroy(struct xa_tracker *xa)
{
free(xa->supported_formats);
xa_context_destroy(xa->default_ctx);
- xa->screen->destroy(xa->screen);
pipe_loader_release(&xa->dev, 1);
free(xa);
}
diff --git a/src/gallium/tests/trivial/compute.c
b/src/gallium/tests/trivial/compute.c
index 443451e13d24..d718906eb7c8 100644
--- a/src/gallium/tests/trivial/compute.c
+++ b/src/gallium/tests/trivial/compute.c
@@ -90,7 +90,6 @@ static void init_ctx(struct context *ctx)
static void destroy_ctx(struct context *ctx)
{
ctx->pipe->destroy(ctx->pipe);
- ctx->screen->destroy(ctx->screen);
pipe_loader_release(&ctx->dev, 1);
FREE(ctx);
}
diff --git a/src/gallium/tests/trivial/quad-tex.c
b/src/gallium/tests/trivial/quad-tex.c
index 2ee544a41294..8b8352d5c535 100644
--- a/src/gallium/tests/trivial/quad-tex.c
+++ b/src/gallium/tests/trivial/quad-tex.c
@@ -291,7 +291,6 @@ static void close_prog(struct program *p)
pipe_resource_reference(&p->vbuf, NULL);
p->pipe->destroy(p->pipe);
- p->screen->destroy(p->screen);
pipe_loader_release(&p->dev, 1);
FREE(p);
diff --git a/src/gallium/tests/trivial/tri.c
b/src/gallium/tests/trivial/tri.c
index a2031696f029..bb053e761b75 100644
--- a/src/gallium/tests/trivial/tri.c
+++ b/src/gallium/tests/trivial/tri.c
@@ -231,7 +231,6 @@ static void close_prog(struct program *p)
pipe_resource_reference(&p->vbuf, NULL);
p->pipe->destroy(p->pipe);
- p->screen->destroy(p->screen);
pipe_loader_release(&p->dev, 1);
FREE(p);
--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Rob Herring
2017-08-07 22:58:01 UTC
Reply
Permalink
Raw Message
Creating a screen needs to be serialized in order to support reusing
existing screen. With this, driver private mutexes in create_screen()
functions can be removed.

pipe_loader_create_screen is made an exported symbol to ensure the same
library function (and mutex) is used when multiple libraries are loaded.

Signed-off-by: Rob Herring <***@kernel.org>
Cc: Emil Velikov <***@gmail.com>
---
src/gallium/auxiliary/pipe-loader/pipe_loader.c | 13 +++++++++++--
src/gallium/targets/dri-vdpau.dyn | 2 ++
src/gallium/targets/dri/dri.sym | 2 ++
src/gallium/targets/vdpau/vdpau.sym | 2 ++
4 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/src/gallium/auxiliary/pipe-loader/pipe_loader.c b/src/gallium/auxiliary/pipe-loader/pipe_loader.c
index 61e5786a2528..4ea3dc81a64a 100644
--- a/src/gallium/auxiliary/pipe-loader/pipe_loader.c
+++ b/src/gallium/auxiliary/pipe-loader/pipe_loader.c
@@ -31,6 +31,7 @@
#include "util/u_memory.h"
#include "util/u_string.h"
#include "util/u_dl.h"
+#include "util/u_thread.h"
#include "util/xmlconfig.h"
#include "util/xmlpool.h"

@@ -43,6 +44,8 @@

#define MODULE_PREFIX "pipe_"

+static mtx_t loader_mutex = _MTX_INITIALIZER_NP;
+
static int (*backends[])(struct pipe_loader_device **, int) = {
#ifdef HAVE_LIBDRM
&pipe_loader_drm_probe,
@@ -70,11 +73,15 @@ pipe_loader_probe(struct pipe_loader_device **devs, int ndev)
static void
pipe_loader_release_dev(struct pipe_loader_device *dev)
{
+ mtx_lock(&loader_mutex);
+
dev->pscreen->destroy(dev->pscreen);
dev->ops->release(&dev);
+
+ mtx_unlock(&loader_mutex);
}

-void
+PUBLIC void
pipe_loader_release(struct pipe_loader_device **devs, int ndev)
{
int i;
@@ -129,7 +136,7 @@ pipe_loader_get_driinfo_xml(const char *driver_name)
return xml;
}

-struct pipe_screen *
+PUBLIC struct pipe_screen *
pipe_loader_create_screen(struct pipe_loader_device *dev)
{
struct pipe_screen *pscreen;
@@ -138,8 +145,10 @@ pipe_loader_create_screen(struct pipe_loader_device *dev)
pipe_loader_load_options(dev);
config.options = &dev->option_cache;

+ mtx_lock(&loader_mutex);
pscreen = dev->ops->create_screen(dev, &config);
dev->pscreen = pscreen;
+ mtx_unlock(&loader_mutex);
return pscreen;
}

diff --git a/src/gallium/targets/dri-vdpau.dyn b/src/gallium/targets/dri-vdpau.dyn
index a7919f7d3ba3..03e9e432b699 100644
--- a/src/gallium/targets/dri-vdpau.dyn
+++ b/src/gallium/targets/dri-vdpau.dyn
@@ -1,4 +1,6 @@
{
+ pipe_loader_create_screen;
+ pipe_loader_release;
nouveau_drm_screen_create;
radeon_drm_winsys_create;
amdgpu_winsys_create;
diff --git a/src/gallium/targets/dri/dri.sym b/src/gallium/targets/dri/dri.sym
index 1fdf18beee76..38b13544ddbf 100644
--- a/src/gallium/targets/dri/dri.sym
+++ b/src/gallium/targets/dri/dri.sym
@@ -2,6 +2,8 @@
global:
__driDriverExtensions;
__driDriverGetExtensions*;
+ pipe_loader_create_screen;
+ pipe_loader_release;
nouveau_drm_screen_create;
radeon_drm_winsys_create;
amdgpu_winsys_create;
diff --git a/src/gallium/targets/vdpau/vdpau.sym b/src/gallium/targets/vdpau/vdpau.sym
index 5e71c6285a64..65b6d1c4ebcc 100644
--- a/src/gallium/targets/vdpau/vdpau.sym
+++ b/src/gallium/targets/vdpau/vdpau.sym
@@ -1,6 +1,8 @@
{
global:
vdp_imp_device_create_x11;
+ pipe_loader_create_screen;
+ pipe_loader_release;
nouveau_drm_screen_create;
radeon_drm_winsys_create;
amdgpu_winsys_create;
--
2.11.0
Rob Herring
2017-08-07 22:58:02 UTC
Reply
Permalink
Raw Message
In order to prevent multiple pipe_screens being created in the same
process, lookup of the DRM FD and reference counting of the pipe_screen
are needed. Several implementations of this exist in various gallium
drivers/winsys already. This creates a common version which is opt-in
for winsys implementations.

The callers of pipe_screen_{un}reference() functions are responsible for
serializing the calls. Currently, this is done by the pipe-loader mutex.

Signed-off-by: Rob Herring <***@kernel.org>
---
src/gallium/auxiliary/Makefile.sources | 2 +
src/gallium/auxiliary/util/u_screen.c | 112 +++++++++++++++++++++++++++++++++
src/gallium/auxiliary/util/u_screen.h | 32 ++++++++++
src/gallium/include/pipe/p_screen.h | 3 +
4 files changed, 149 insertions(+)
create mode 100644 src/gallium/auxiliary/util/u_screen.c
create mode 100644 src/gallium/auxiliary/util/u_screen.h

diff --git a/src/gallium/auxiliary/Makefile.sources b/src/gallium/auxiliary/Makefile.sources
index 9ae8e6c8ca53..f84bfec60fe7 100644
--- a/src/gallium/auxiliary/Makefile.sources
+++ b/src/gallium/auxiliary/Makefile.sources
@@ -283,6 +283,8 @@ C_SOURCES := \
util/u_ringbuffer.h \
util/u_sampler.c \
util/u_sampler.h \
+ util/u_screen.c \
+ util/u_screen.h \
util/u_simple_shaders.c \
util/u_simple_shaders.h \
util/u_split_prim.h \
diff --git a/src/gallium/auxiliary/util/u_screen.c b/src/gallium/auxiliary/util/u_screen.c
new file mode 100644
index 000000000000..dec83b736883
--- /dev/null
+++ b/src/gallium/auxiliary/util/u_screen.c
@@ -0,0 +1,112 @@
+/*
+ * Copyright 2016-2017 Linaro, Ltd., Rob Herring <***@kernel.org>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the
+ * "Software"), to deal in the Software without restriction, including
+ * without limitation the rights to use, copy, modify, merge, publish,
+ * distribute, sub license, and/or sell copies of the Software, and to
+ * permit persons to whom the Software is furnished to do so, subject to
+ * the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the
+ * next paragraph) shall be included in all copies or substantial portions
+ * of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
+ * OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT.
+ * IN NO EVENT SHALL VMWARE AND/OR ITS SUPPLIERS BE LIABLE FOR
+ * ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,
+ * TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
+ * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+/**
+ * Functions for managing pipe_screen's
+ */
+
+#include <sys/stat.h>
+
+#include "pipe/p_screen.h"
+#include "util/u_hash_table.h"
+#include "util/u_inlines.h"
+#include "util/u_pointer.h"
+#include "util/u_screen.h"
+
+static struct util_hash_table *fd_tab = NULL;
+
+static unsigned hash_fd(void *key)
+{
+ int fd = pointer_to_intptr(key);
+ struct stat stat;
+ fstat(fd, &stat);
+
+ return stat.st_dev ^ stat.st_ino ^ stat.st_rdev;
+}
+
+static int compare_fd(void *key1, void *key2)
+{
+ int fd1 = pointer_to_intptr(key1);
+ int fd2 = pointer_to_intptr(key2);
+ struct stat stat1, stat2;
+ fstat(fd1, &stat1);
+ fstat(fd2, &stat2);
+
+ return stat1.st_dev != stat2.st_dev ||
+ stat1.st_ino != stat2.st_ino ||
+ stat1.st_rdev != stat2.st_rdev;
+}
+
+struct pipe_screen *
+pipe_screen_reference(int fd)
+{
+ struct pipe_screen *pscreen;
+
+ if (!fd_tab) {
+ fd_tab = util_hash_table_create(hash_fd, compare_fd);
+ return NULL;
+ }
+
+ pscreen = util_hash_table_get(fd_tab, intptr_to_pointer(fd));
+ if (pscreen)
+ pipe_reference(NULL, &pscreen->reference);
+
+ return pscreen;
+}
+
+boolean
+pipe_screen_unreference(struct pipe_screen *pscreen)
+{
+ boolean destroy;
+
+ if (!pscreen)
+ return FALSE;
+
+ /* Work-around until all pipe_screens have ref counting */
+ if (!pipe_is_referenced(&pscreen->reference)) {
+ pscreen->destroy(pscreen);
+ return TRUE;
+ }
+
+ destroy = pipe_reference(&pscreen->reference, NULL);
+ if (destroy) {
+ int fd = pscreen->fd;
+
+ pscreen->destroy(pscreen);
+
+ if (fd >= 0) {
+ util_hash_table_remove(fd_tab, intptr_to_pointer(fd));
+ close(fd);
+ }
+ }
+ return destroy;
+}
+
+
+void pipe_screen_reference_init(struct pipe_screen *pscreen, int fd)
+{
+ pscreen->fd = fd;
+ pipe_reference_init(&pscreen->reference, 1);
+ util_hash_table_set(fd_tab, intptr_to_pointer(fd), pscreen);
+}
diff --git a/src/gallium/auxiliary/util/u_screen.h b/src/gallium/auxiliary/util/u_screen.h
new file mode 100644
index 000000000000..a7b1e9ff2787
--- /dev/null
+++ b/src/gallium/auxiliary/util/u_screen.h
@@ -0,0 +1,32 @@
+/*
+ * Copyright 2016-2017 Linaro, Ltd., Rob Herring <***@kernel.org>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the
+ * "Software"), to deal in the Software without restriction, including
+ * without limitation the rights to use, copy, modify, merge, publish,
+ * distribute, sub license, and/or sell copies of the Software, and to
+ * permit persons to whom the Software is furnished to do so, subject to
+ * the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the
+ * next paragraph) shall be included in all copies or substantial portions
+ * of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
+ * OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT.
+ * IN NO EVENT SHALL VMWARE AND/OR ITS SUPPLIERS BE LIABLE FOR
+ * ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,
+ * TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
+ * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+ */
+#include "pipe/p_defines.h"
+
+struct pipe_screen;
+
+struct pipe_screen *pipe_screen_reference(int fd);
+
+boolean pipe_screen_unreference(struct pipe_screen *pscreen);
+
+void pipe_screen_reference_init(struct pipe_screen *pscreen, int fd);
diff --git a/src/gallium/include/pipe/p_screen.h b/src/gallium/include/pipe/p_screen.h
index b6623d1dd71e..dbb14ef882c0 100644
--- a/src/gallium/include/pipe/p_screen.h
+++ b/src/gallium/include/pipe/p_screen.h
@@ -41,6 +41,7 @@
#include "pipe/p_compiler.h"
#include "pipe/p_format.h"
#include "pipe/p_defines.h"
+#include "pipe/p_state.h"
#include "pipe/p_video_enums.h"


@@ -68,6 +69,8 @@ struct driOptionCache;
* context.
*/
struct pipe_screen {
+ struct pipe_reference reference;
+ int fd;
void (*destroy)( struct pipe_screen * );

const char *(*get_name)( struct pipe_screen * );
--
2.11.0
Marek Olšák
2017-08-09 19:18:01 UTC
Reply
Permalink
Raw Message
Hi Rob,

This is not enough to do screen reference counting. There are primary
FDs and render node FDs, and we want to have only one pipe_screen for
all kinds of FDs pointing to the same device.

Imagine someone creates a pipe_screen with a render node FD, and then
creates another pipe_screen with a primary FD. We want to return the
same pipe_screen instance in both cases.

The pipe_screen should use the render node FD for everything if it was
created first with that FD. However, it also needs to keep the primary
FD in order to be able to do GEM_FLINK when required.

libdrm_amdgpu handles all those cases correctly. It has "fd", which is
the main one, but also has "flink_fd" for GEM_FLINK. If "fd" is a
primary FD or there is no primary FD, "fd == flink_fd". If "fd" is a
render node and a primary FD has also been used to create the device
object, "fd" is the render node and "flink_fd" is the primary FD for
GEM_FLINK.

The code:
https://cgit.freedesktop.org/mesa/drm/tree/amdgpu/amdgpu_device.c

I can't accept this patch, because it effectively disables that
amdgpu_device code.

Marek
Post by Rob Herring
In order to prevent multiple pipe_screens being created in the same
process, lookup of the DRM FD and reference counting of the pipe_screen
are needed. Several implementations of this exist in various gallium
drivers/winsys already. This creates a common version which is opt-in
for winsys implementations.
The callers of pipe_screen_{un}reference() functions are responsible for
serializing the calls. Currently, this is done by the pipe-loader mutex.
---
src/gallium/auxiliary/Makefile.sources | 2 +
src/gallium/auxiliary/util/u_screen.c | 112 +++++++++++++++++++++++++++++++++
src/gallium/auxiliary/util/u_screen.h | 32 ++++++++++
src/gallium/include/pipe/p_screen.h | 3 +
4 files changed, 149 insertions(+)
create mode 100644 src/gallium/auxiliary/util/u_screen.c
create mode 100644 src/gallium/auxiliary/util/u_screen.h
diff --git a/src/gallium/auxiliary/Makefile.sources b/src/gallium/auxiliary/Makefile.sources
index 9ae8e6c8ca53..f84bfec60fe7 100644
--- a/src/gallium/auxiliary/Makefile.sources
+++ b/src/gallium/auxiliary/Makefile.sources
@@ -283,6 +283,8 @@ C_SOURCES := \
util/u_ringbuffer.h \
util/u_sampler.c \
util/u_sampler.h \
+ util/u_screen.c \
+ util/u_screen.h \
util/u_simple_shaders.c \
util/u_simple_shaders.h \
util/u_split_prim.h \
diff --git a/src/gallium/auxiliary/util/u_screen.c b/src/gallium/auxiliary/util/u_screen.c
new file mode 100644
index 000000000000..dec83b736883
--- /dev/null
+++ b/src/gallium/auxiliary/util/u_screen.c
@@ -0,0 +1,112 @@
+/*
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the
+ * "Software"), to deal in the Software without restriction, including
+ * without limitation the rights to use, copy, modify, merge, publish,
+ * distribute, sub license, and/or sell copies of the Software, and to
+ * permit persons to whom the Software is furnished to do so, subject to
+ *
+ * The above copyright notice and this permission notice (including the
+ * next paragraph) shall be included in all copies or substantial portions
+ * of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
+ * OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT.
+ * IN NO EVENT SHALL VMWARE AND/OR ITS SUPPLIERS BE LIABLE FOR
+ * ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,
+ * TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
+ * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+/**
+ * Functions for managing pipe_screen's
+ */
+
+#include <sys/stat.h>
+
+#include "pipe/p_screen.h"
+#include "util/u_hash_table.h"
+#include "util/u_inlines.h"
+#include "util/u_pointer.h"
+#include "util/u_screen.h"
+
+static struct util_hash_table *fd_tab = NULL;
+
+static unsigned hash_fd(void *key)
+{
+ int fd = pointer_to_intptr(key);
+ struct stat stat;
+ fstat(fd, &stat);
+
+ return stat.st_dev ^ stat.st_ino ^ stat.st_rdev;
+}
+
+static int compare_fd(void *key1, void *key2)
+{
+ int fd1 = pointer_to_intptr(key1);
+ int fd2 = pointer_to_intptr(key2);
+ struct stat stat1, stat2;
+ fstat(fd1, &stat1);
+ fstat(fd2, &stat2);
+
+ return stat1.st_dev != stat2.st_dev ||
+ stat1.st_ino != stat2.st_ino ||
+ stat1.st_rdev != stat2.st_rdev;
+}
+
+struct pipe_screen *
+pipe_screen_reference(int fd)
+{
+ struct pipe_screen *pscreen;
+
+ if (!fd_tab) {
+ fd_tab = util_hash_table_create(hash_fd, compare_fd);
+ return NULL;
+ }
+
+ pscreen = util_hash_table_get(fd_tab, intptr_to_pointer(fd));
+ if (pscreen)
+ pipe_reference(NULL, &pscreen->reference);
+
+ return pscreen;
+}
+
+boolean
+pipe_screen_unreference(struct pipe_screen *pscreen)
+{
+ boolean destroy;
+
+ if (!pscreen)
+ return FALSE;
+
+ /* Work-around until all pipe_screens have ref counting */
+ if (!pipe_is_referenced(&pscreen->reference)) {
+ pscreen->destroy(pscreen);
+ return TRUE;
+ }
+
+ destroy = pipe_reference(&pscreen->reference, NULL);
+ if (destroy) {
+ int fd = pscreen->fd;
+
+ pscreen->destroy(pscreen);
+
+ if (fd >= 0) {
+ util_hash_table_remove(fd_tab, intptr_to_pointer(fd));
+ close(fd);
+ }
+ }
+ return destroy;
+}
+
+
+void pipe_screen_reference_init(struct pipe_screen *pscreen, int fd)
+{
+ pscreen->fd = fd;
+ pipe_reference_init(&pscreen->reference, 1);
+ util_hash_table_set(fd_tab, intptr_to_pointer(fd), pscreen);
+}
diff --git a/src/gallium/auxiliary/util/u_screen.h b/src/gallium/auxiliary/util/u_screen.h
new file mode 100644
index 000000000000..a7b1e9ff2787
--- /dev/null
+++ b/src/gallium/auxiliary/util/u_screen.h
@@ -0,0 +1,32 @@
+/*
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the
+ * "Software"), to deal in the Software without restriction, including
+ * without limitation the rights to use, copy, modify, merge, publish,
+ * distribute, sub license, and/or sell copies of the Software, and to
+ * permit persons to whom the Software is furnished to do so, subject to
+ *
+ * The above copyright notice and this permission notice (including the
+ * next paragraph) shall be included in all copies or substantial portions
+ * of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
+ * OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT.
+ * IN NO EVENT SHALL VMWARE AND/OR ITS SUPPLIERS BE LIABLE FOR
+ * ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,
+ * TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
+ * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+ */
+#include "pipe/p_defines.h"
+
+struct pipe_screen;
+
+struct pipe_screen *pipe_screen_reference(int fd);
+
+boolean pipe_screen_unreference(struct pipe_screen *pscreen);
+
+void pipe_screen_reference_init(struct pipe_screen *pscreen, int fd);
diff --git a/src/gallium/include/pipe/p_screen.h b/src/gallium/include/pipe/p_screen.h
index b6623d1dd71e..dbb14ef882c0 100644
--- a/src/gallium/include/pipe/p_screen.h
+++ b/src/gallium/include/pipe/p_screen.h
@@ -41,6 +41,7 @@
#include "pipe/p_compiler.h"
#include "pipe/p_format.h"
#include "pipe/p_defines.h"
+#include "pipe/p_state.h"
#include "pipe/p_video_enums.h"
@@ -68,6 +69,8 @@ struct driOptionCache;
* context.
*/
struct pipe_screen {
+ struct pipe_reference reference;
+ int fd;
void (*destroy)( struct pipe_screen * );
const char *(*get_name)( struct pipe_screen * );
--
2.11.0
_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Rob Herring
2017-08-09 19:53:55 UTC
Reply
Permalink
Raw Message
Post by Marek Olšák
Hi Rob,
This is not enough to do screen reference counting. There are primary
FDs and render node FDs, and we want to have only one pipe_screen for
all kinds of FDs pointing to the same device.
Imagine someone creates a pipe_screen with a render node FD, and then
creates another pipe_screen with a primary FD. We want to return the
same pipe_screen instance in both cases.
The pipe_screen should use the render node FD for everything if it was
created first with that FD. However, it also needs to keep the primary
FD in order to be able to do GEM_FLINK when required.
libdrm_amdgpu handles all those cases correctly. It has "fd", which is
the main one, but also has "flink_fd" for GEM_FLINK. If "fd" is a
primary FD or there is no primary FD, "fd == flink_fd". If "fd" is a
render node and a primary FD has also been used to create the device
object, "fd" is the render node and "flink_fd" is the primary FD for
GEM_FLINK.
https://cgit.freedesktop.org/mesa/drm/tree/amdgpu/amdgpu_device.c
I can't accept this patch, because it effectively disables that
amdgpu_device code.
This patch alone doesn't change any driver. I think you mean patch #8
which converts amdgpu and which could be dropped (though I'd like to
keep the radeon winsys parts). However, I believe I've maintained the
behavior for amdgpu.

The amdgpu winsys does not use the fd in these functions (the screen
fd will be -1). The only change should be that the mutex is removed
and the pipe_reference is moved from ws->reference to
ws->base.screen->reference. It's still using the device in the hash
table.

Rob
Marek Olšák
2017-08-09 20:12:12 UTC
Reply
Permalink
Raw Message
OK. This patch is:

Reviewed-by: Marek Olšák <***@amd.com>

Marek
Post by Rob Herring
Post by Marek Olšák
Hi Rob,
This is not enough to do screen reference counting. There are primary
FDs and render node FDs, and we want to have only one pipe_screen for
all kinds of FDs pointing to the same device.
Imagine someone creates a pipe_screen with a render node FD, and then
creates another pipe_screen with a primary FD. We want to return the
same pipe_screen instance in both cases.
The pipe_screen should use the render node FD for everything if it was
created first with that FD. However, it also needs to keep the primary
FD in order to be able to do GEM_FLINK when required.
libdrm_amdgpu handles all those cases correctly. It has "fd", which is
the main one, but also has "flink_fd" for GEM_FLINK. If "fd" is a
primary FD or there is no primary FD, "fd == flink_fd". If "fd" is a
render node and a primary FD has also been used to create the device
object, "fd" is the render node and "flink_fd" is the primary FD for
GEM_FLINK.
https://cgit.freedesktop.org/mesa/drm/tree/amdgpu/amdgpu_device.c
I can't accept this patch, because it effectively disables that
amdgpu_device code.
This patch alone doesn't change any driver. I think you mean patch #8
which converts amdgpu and which could be dropped (though I'd like to
keep the radeon winsys parts). However, I believe I've maintained the
behavior for amdgpu.
The amdgpu winsys does not use the fd in these functions (the screen
fd will be -1). The only change should be that the mutex is removed
and the pipe_reference is moved from ws->reference to
ws->base.screen->reference. It's still using the device in the hash
table.
Rob
Marek Olšák
2017-08-09 20:18:06 UTC
Reply
Permalink
Raw Message
Post by Rob Herring
In order to prevent multiple pipe_screens being created in the same
process, lookup of the DRM FD and reference counting of the pipe_screen
are needed. Several implementations of this exist in various gallium
drivers/winsys already. This creates a common version which is opt-in
for winsys implementations.
The callers of pipe_screen_{un}reference() functions are responsible for
serializing the calls. Currently, this is done by the pipe-loader mutex.
---
src/gallium/auxiliary/Makefile.sources | 2 +
src/gallium/auxiliary/util/u_screen.c | 112 +++++++++++++++++++++++++++++++++
src/gallium/auxiliary/util/u_screen.h | 32 ++++++++++
src/gallium/include/pipe/p_screen.h | 3 +
4 files changed, 149 insertions(+)
create mode 100644 src/gallium/auxiliary/util/u_screen.c
create mode 100644 src/gallium/auxiliary/util/u_screen.h
diff --git a/src/gallium/auxiliary/Makefile.sources b/src/gallium/auxiliary/Makefile.sources
index 9ae8e6c8ca53..f84bfec60fe7 100644
--- a/src/gallium/auxiliary/Makefile.sources
+++ b/src/gallium/auxiliary/Makefile.sources
@@ -283,6 +283,8 @@ C_SOURCES := \
util/u_ringbuffer.h \
util/u_sampler.c \
util/u_sampler.h \
+ util/u_screen.c \
+ util/u_screen.h \
util/u_simple_shaders.c \
util/u_simple_shaders.h \
util/u_split_prim.h \
diff --git a/src/gallium/auxiliary/util/u_screen.c b/src/gallium/auxiliary/util/u_screen.c
new file mode 100644
index 000000000000..dec83b736883
--- /dev/null
+++ b/src/gallium/auxiliary/util/u_screen.c
@@ -0,0 +1,112 @@
+/*
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the
+ * "Software"), to deal in the Software without restriction, including
+ * without limitation the rights to use, copy, modify, merge, publish,
+ * distribute, sub license, and/or sell copies of the Software, and to
+ * permit persons to whom the Software is furnished to do so, subject to
+ *
+ * The above copyright notice and this permission notice (including the
+ * next paragraph) shall be included in all copies or substantial portions
+ * of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
+ * OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT.
+ * IN NO EVENT SHALL VMWARE AND/OR ITS SUPPLIERS BE LIABLE FOR
+ * ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,
+ * TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
+ * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+/**
+ * Functions for managing pipe_screen's
+ */
+
+#include <sys/stat.h>
+
+#include "pipe/p_screen.h"
+#include "util/u_hash_table.h"
+#include "util/u_inlines.h"
+#include "util/u_pointer.h"
+#include "util/u_screen.h"
+
+static struct util_hash_table *fd_tab = NULL;
+
+static unsigned hash_fd(void *key)
+{
+ int fd = pointer_to_intptr(key);
+ struct stat stat;
+ fstat(fd, &stat);
+
+ return stat.st_dev ^ stat.st_ino ^ stat.st_rdev;
+}
+
+static int compare_fd(void *key1, void *key2)
+{
+ int fd1 = pointer_to_intptr(key1);
+ int fd2 = pointer_to_intptr(key2);
+ struct stat stat1, stat2;
+ fstat(fd1, &stat1);
+ fstat(fd2, &stat2);
+
+ return stat1.st_dev != stat2.st_dev ||
+ stat1.st_ino != stat2.st_ino ||
+ stat1.st_rdev != stat2.st_rdev;
+}
+
+struct pipe_screen *
+pipe_screen_reference(int fd)
+{
+ struct pipe_screen *pscreen;
+
+ if (!fd_tab) {
+ fd_tab = util_hash_table_create(hash_fd, compare_fd);
+ return NULL;
+ }
+
+ pscreen = util_hash_table_get(fd_tab, intptr_to_pointer(fd));
+ if (pscreen)
+ pipe_reference(NULL, &pscreen->reference);
+
+ return pscreen;
+}
+
+boolean
+pipe_screen_unreference(struct pipe_screen *pscreen)
+{
+ boolean destroy;
+
+ if (!pscreen)
+ return FALSE;
+
+ /* Work-around until all pipe_screens have ref counting */
+ if (!pipe_is_referenced(&pscreen->reference)) {
+ pscreen->destroy(pscreen);
+ return TRUE;
+ }
+
+ destroy = pipe_reference(&pscreen->reference, NULL);
+ if (destroy) {
+ int fd = pscreen->fd;
+
+ pscreen->destroy(pscreen);
+
+ if (fd >= 0) {
+ util_hash_table_remove(fd_tab, intptr_to_pointer(fd));
+ close(fd);
+ }
+ }
+ return destroy;
+}
+
+
+void pipe_screen_reference_init(struct pipe_screen *pscreen, int fd)
+{
+ pscreen->fd = fd;
+ pipe_reference_init(&pscreen->reference, 1);
+ util_hash_table_set(fd_tab, intptr_to_pointer(fd), pscreen);
+}
diff --git a/src/gallium/auxiliary/util/u_screen.h b/src/gallium/auxiliary/util/u_screen.h
new file mode 100644
index 000000000000..a7b1e9ff2787
--- /dev/null
+++ b/src/gallium/auxiliary/util/u_screen.h
@@ -0,0 +1,32 @@
+/*
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the
+ * "Software"), to deal in the Software without restriction, including
+ * without limitation the rights to use, copy, modify, merge, publish,
+ * distribute, sub license, and/or sell copies of the Software, and to
+ * permit persons to whom the Software is furnished to do so, subject to
+ *
+ * The above copyright notice and this permission notice (including the
+ * next paragraph) shall be included in all copies or substantial portions
+ * of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
+ * OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT.
+ * IN NO EVENT SHALL VMWARE AND/OR ITS SUPPLIERS BE LIABLE FOR
+ * ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,
+ * TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
+ * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+ */
+#include "pipe/p_defines.h"
+
+struct pipe_screen;
+
+struct pipe_screen *pipe_screen_reference(int fd);
+
+boolean pipe_screen_unreference(struct pipe_screen *pscreen);
+
+void pipe_screen_reference_init(struct pipe_screen *pscreen, int fd);
diff --git a/src/gallium/include/pipe/p_screen.h b/src/gallium/include/pipe/p_screen.h
index b6623d1dd71e..dbb14ef882c0 100644
--- a/src/gallium/include/pipe/p_screen.h
+++ b/src/gallium/include/pipe/p_screen.h
@@ -41,6 +41,7 @@
#include "pipe/p_compiler.h"
#include "pipe/p_format.h"
#include "pipe/p_defines.h"
+#include "pipe/p_state.h"
#include "pipe/p_video_enums.h"
@@ -68,6 +69,8 @@ struct driOptionCache;
* context.
*/
struct pipe_screen {
+ struct pipe_reference reference;
+ int fd;
void (*destroy)( struct pipe_screen * );
BTW, I'd like an empty line between "fd" and "destroy".

Marek
Rob Herring
2017-08-07 22:58:03 UTC
Reply
Permalink
Raw Message
Use pipe_screen_unreference as it will call pipe_screen->destroy() when
the pipe_screen is no longer referenced.

Signed-off-by: Rob Herring <***@kernel.org>
---
src/gallium/drivers/ddebug/dd_screen.c | 3 ++-
src/gallium/drivers/noop/noop_pipe.c | 3 ++-
src/gallium/drivers/rbug/rbug_screen.c | 3 ++-
src/gallium/drivers/trace/tr_screen.c | 3 ++-
4 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/src/gallium/drivers/ddebug/dd_screen.c b/src/gallium/drivers/ddebug/dd_screen.c
index 14e6f6b011f7..806846573234 100644
--- a/src/gallium/drivers/ddebug/dd_screen.c
+++ b/src/gallium/drivers/ddebug/dd_screen.c
@@ -28,6 +28,7 @@
#include "dd_pipe.h"
#include "dd_public.h"
#include "util/u_memory.h"
+#include "util/u_screen.h"
#include <stdio.h>


@@ -314,7 +315,7 @@ dd_screen_destroy(struct pipe_screen *_screen)
struct dd_screen *dscreen = dd_screen(_screen);
struct pipe_screen *screen = dscreen->screen;

- screen->destroy(screen);
+ pipe_screen_unreference(screen);
FREE(dscreen);
}

diff --git a/src/gallium/drivers/noop/noop_pipe.c b/src/gallium/drivers/noop/noop_pipe.c
index d1e795dab163..28c095058a36 100644
--- a/src/gallium/drivers/noop/noop_pipe.c
+++ b/src/gallium/drivers/noop/noop_pipe.c
@@ -29,6 +29,7 @@
#include "util/u_memory.h"
#include "util/u_inlines.h"
#include "util/u_format.h"
+#include "util/u_screen.h"
#include "util/u_upload_mgr.h"
#include "noop_public.h"

@@ -431,7 +432,7 @@ static void noop_destroy_screen(struct pipe_screen *screen)
struct noop_pipe_screen *noop_screen = (struct noop_pipe_screen*)screen;
struct pipe_screen *oscreen = noop_screen->oscreen;

- oscreen->destroy(oscreen);
+ pipe_screen_unreference(oscreen);
FREE(screen);
}

diff --git a/src/gallium/drivers/rbug/rbug_screen.c b/src/gallium/drivers/rbug/rbug_screen.c
index b12f029b3ea1..dc36425cc45f 100644
--- a/src/gallium/drivers/rbug/rbug_screen.c
+++ b/src/gallium/drivers/rbug/rbug_screen.c
@@ -30,6 +30,7 @@
#include "pipe/p_state.h"
#include "util/u_memory.h"
#include "util/u_debug.h"
+#include "util/u_screen.h"
#include "util/simple_list.h"

#include "rbug_public.h"
@@ -45,7 +46,7 @@ rbug_screen_destroy(struct pipe_screen *_screen)
struct rbug_screen *rb_screen = rbug_screen(_screen);
struct pipe_screen *screen = rb_screen->screen;

- screen->destroy(screen);
+ pipe_screen_unreference(screen);

FREE(rb_screen);
}
diff --git a/src/gallium/drivers/trace/tr_screen.c b/src/gallium/drivers/trace/tr_screen.c
index e56434c5bda6..697854185d54 100644
--- a/src/gallium/drivers/trace/tr_screen.c
+++ b/src/gallium/drivers/trace/tr_screen.c
@@ -27,6 +27,7 @@

#include "util/u_format.h"
#include "util/u_memory.h"
+#include "util/u_screen.h"
#include "util/simple_list.h"

#include "tr_dump.h"
@@ -488,7 +489,7 @@ trace_screen_destroy(struct pipe_screen *_screen)
trace_dump_arg(ptr, screen);
trace_dump_call_end();

- screen->destroy(screen);
+ pipe_screen_unreference(screen);

FREE(tr_scr);
}
--
2.11.0
Marek Olšák
2017-08-09 20:14:30 UTC
Reply
Permalink
Raw Message
Reviewed-by: Marek Olšák <***@amd.com>

Marek
Post by Rob Herring
Use pipe_screen_unreference as it will call pipe_screen->destroy() when
the pipe_screen is no longer referenced.
---
src/gallium/drivers/ddebug/dd_screen.c | 3 ++-
src/gallium/drivers/noop/noop_pipe.c | 3 ++-
src/gallium/drivers/rbug/rbug_screen.c | 3 ++-
src/gallium/drivers/trace/tr_screen.c | 3 ++-
4 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/src/gallium/drivers/ddebug/dd_screen.c b/src/gallium/drivers/ddebug/dd_screen.c
index 14e6f6b011f7..806846573234 100644
--- a/src/gallium/drivers/ddebug/dd_screen.c
+++ b/src/gallium/drivers/ddebug/dd_screen.c
@@ -28,6 +28,7 @@
#include "dd_pipe.h"
#include "dd_public.h"
#include "util/u_memory.h"
+#include "util/u_screen.h"
#include <stdio.h>
@@ -314,7 +315,7 @@ dd_screen_destroy(struct pipe_screen *_screen)
struct dd_screen *dscreen = dd_screen(_screen);
struct pipe_screen *screen = dscreen->screen;
- screen->destroy(screen);
+ pipe_screen_unreference(screen);
FREE(dscreen);
}
diff --git a/src/gallium/drivers/noop/noop_pipe.c b/src/gallium/drivers/noop/noop_pipe.c
index d1e795dab163..28c095058a36 100644
--- a/src/gallium/drivers/noop/noop_pipe.c
+++ b/src/gallium/drivers/noop/noop_pipe.c
@@ -29,6 +29,7 @@
#include "util/u_memory.h"
#include "util/u_inlines.h"
#include "util/u_format.h"
+#include "util/u_screen.h"
#include "util/u_upload_mgr.h"
#include "noop_public.h"
@@ -431,7 +432,7 @@ static void noop_destroy_screen(struct pipe_screen *screen)
struct noop_pipe_screen *noop_screen = (struct noop_pipe_screen*)screen;
struct pipe_screen *oscreen = noop_screen->oscreen;
- oscreen->destroy(oscreen);
+ pipe_screen_unreference(oscreen);
FREE(screen);
}
diff --git a/src/gallium/drivers/rbug/rbug_screen.c b/src/gallium/drivers/rbug/rbug_screen.c
index b12f029b3ea1..dc36425cc45f 100644
--- a/src/gallium/drivers/rbug/rbug_screen.c
+++ b/src/gallium/drivers/rbug/rbug_screen.c
@@ -30,6 +30,7 @@
#include "pipe/p_state.h"
#include "util/u_memory.h"
#include "util/u_debug.h"
+#include "util/u_screen.h"
#include "util/simple_list.h"
#include "rbug_public.h"
@@ -45,7 +46,7 @@ rbug_screen_destroy(struct pipe_screen *_screen)
struct rbug_screen *rb_screen = rbug_screen(_screen);
struct pipe_screen *screen = rb_screen->screen;
- screen->destroy(screen);
+ pipe_screen_unreference(screen);
FREE(rb_screen);
}
diff --git a/src/gallium/drivers/trace/tr_screen.c b/src/gallium/drivers/trace/tr_screen.c
index e56434c5bda6..697854185d54 100644
--- a/src/gallium/drivers/trace/tr_screen.c
+++ b/src/gallium/drivers/trace/tr_screen.c
@@ -27,6 +27,7 @@
#include "util/u_format.h"
#include "util/u_memory.h"
+#include "util/u_screen.h"
#include "util/simple_list.h"
#include "tr_dump.h"
@@ -488,7 +489,7 @@ trace_screen_destroy(struct pipe_screen *_screen)
trace_dump_arg(ptr, screen);
trace_dump_call_end();
- screen->destroy(screen);
+ pipe_screen_unreference(screen);
FREE(tr_scr);
}
--
2.11.0
_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Rob Herring
2017-08-07 22:58:07 UTC
Reply
Permalink
Raw Message
Use the common pipe_screen ref counting and fd hashing functions.
The mutex can be dropped as the pipe loader serializes the
create_screen() and destroy() calls.

Signed-off-by: Rob Herring <***@kernel.org>
Cc: Christian Gmeiner <***@gmail.com>
Cc: Wladimir J. van der Laan <***@gmail.com>
Cc: Lucas Stach <***@pengutronix.de>
---
src/gallium/drivers/etnaviv/etnaviv_screen.c | 1 -
src/gallium/drivers/etnaviv/etnaviv_screen.h | 4 --
.../winsys/etnaviv/drm/etnaviv_drm_winsys.c | 81 +++-------------------
3 files changed, 8 insertions(+), 78 deletions(-)

diff --git a/src/gallium/drivers/etnaviv/etnaviv_screen.c b/src/gallium/drivers/etnaviv/etnaviv_screen.c
index 9aca90642c37..92950c3bfbb5 100644
--- a/src/gallium/drivers/etnaviv/etnaviv_screen.c
+++ b/src/gallium/drivers/etnaviv/etnaviv_screen.c
@@ -806,7 +806,6 @@ etna_screen_create(struct etna_device *dev, struct etna_gpu *gpu,
screen->dev = dev;
screen->gpu = gpu;
screen->ro = renderonly_dup(ro);
- screen->refcnt = 1;

if (!screen->ro) {
DBG("could not create renderonly object");
diff --git a/src/gallium/drivers/etnaviv/etnaviv_screen.h b/src/gallium/drivers/etnaviv/etnaviv_screen.h
index dc57a38dbb80..74b8e98d1695 100644
--- a/src/gallium/drivers/etnaviv/etnaviv_screen.h
+++ b/src/gallium/drivers/etnaviv/etnaviv_screen.h
@@ -30,7 +30,6 @@

#include "etnaviv_internal.h"

-#include "os/os_thread.h"
#include "pipe/p_screen.h"
#include "renderonly/renderonly.h"
#include "util/slab.h"
@@ -59,9 +58,6 @@ enum viv_features_word {
struct etna_screen {
struct pipe_screen base;

- int refcnt;
- void *winsys_priv;
-
struct etna_device *dev;
struct etna_gpu *gpu;
struct etna_pipe *pipe;
diff --git a/src/gallium/winsys/etnaviv/drm/etnaviv_drm_winsys.c b/src/gallium/winsys/etnaviv/drm/etnaviv_drm_winsys.c
index 8e3f7a06a9a0..09b20389810b 100644
--- a/src/gallium/winsys/etnaviv/drm/etnaviv_drm_winsys.c
+++ b/src/gallium/winsys/etnaviv/drm/etnaviv_drm_winsys.c
@@ -28,6 +28,7 @@

#include "util/u_hash_table.h"
#include "util/u_memory.h"
+#include "util/u_screen.h"

#include "etnaviv/etnaviv_screen.h"
#include "etnaviv/hw/common.xml.h"
@@ -67,85 +68,19 @@ screen_create(struct renderonly *ro)
return etna_screen_create(dev, gpu, ro);
}

-static struct util_hash_table *etna_tab = NULL;
-
-static mtx_t etna_screen_mutex = _MTX_INITIALIZER_NP;
-
-static void
-etna_drm_screen_destroy(struct pipe_screen *pscreen)
-{
- struct etna_screen *screen = etna_screen(pscreen);
- boolean destroy;
-
- mtx_lock(&etna_screen_mutex);
- destroy = --screen->refcnt == 0;
- if (destroy) {
- int fd = etna_device_fd(screen->dev);
- util_hash_table_remove(etna_tab, intptr_to_pointer(fd));
- }
- mtx_unlock(&etna_screen_mutex);
-
- if (destroy) {
- pscreen->destroy = screen->winsys_priv;
- pscreen->destroy(pscreen);
- }
-}
-
-static unsigned hash_fd(void *key)
-{
- int fd = pointer_to_intptr(key);
- struct stat stat;
-
- fstat(fd, &stat);
-
- return stat.st_dev ^ stat.st_ino ^ stat.st_rdev;
-}
-
-static int compare_fd(void *key1, void *key2)
-{
- int fd1 = pointer_to_intptr(key1);
- int fd2 = pointer_to_intptr(key2);
- struct stat stat1, stat2;
-
- fstat(fd1, &stat1);
- fstat(fd2, &stat2);
-
- return stat1.st_dev != stat2.st_dev ||
- stat1.st_ino != stat2.st_ino ||
- stat1.st_rdev != stat2.st_rdev;
-}
-
struct pipe_screen *
etna_drm_screen_create_renderonly(struct renderonly *ro)
{
- struct pipe_screen *pscreen = NULL;
+ struct etna_screen *screen;
+ struct pipe_screen *pscreen = pipe_screen_reference(ro->gpu_fd);

- mtx_lock(&etna_screen_mutex);
- if (!etna_tab) {
- etna_tab = util_hash_table_create(hash_fd, compare_fd);
- if (!etna_tab)
- goto unlock;
- }
+ if (pscreen)
+ return pscreen;

- pscreen = util_hash_table_get(etna_tab, intptr_to_pointer(ro->gpu_fd));
- if (pscreen) {
- etna_screen(pscreen)->refcnt++;
- } else {
- pscreen = screen_create(ro);
- if (pscreen) {
- int fd = etna_device_fd(etna_screen(pscreen)->dev);
- util_hash_table_set(etna_tab, intptr_to_pointer(fd), pscreen);
-
- /* Bit of a hack, to avoid circular linkage dependency,
- * ie. pipe driver having to call in to winsys, we
- * override the pipe drivers screen->destroy() */
- etna_screen(pscreen)->winsys_priv = pscreen->destroy;
- pscreen->destroy = etna_drm_screen_destroy;
- }
- }
+ pscreen = screen_create(ro);

-unlock:
- mtx_unlock(&etna_screen_mutex);
+ screen = etna_screen(pscreen);
+ pipe_screen_reference_init(pscreen, etna_device_fd(screen->dev));
return pscreen;
}
--
2.11.0
Wladimir
2017-08-08 09:49:06 UTC
Reply
Permalink
Raw Message
Post by Rob Herring
Use the common pipe_screen ref counting and fd hashing functions.
The mutex can be dropped as the pipe loader serializes the
create_screen() and destroy() calls.
This sure cleans up a lot.

Reviewed-by: Wladimir J. van der Laan <***@gmail.com>
(will test)
Post by Rob Herring
---
src/gallium/drivers/etnaviv/etnaviv_screen.c | 1 -
src/gallium/drivers/etnaviv/etnaviv_screen.h | 4 --
.../winsys/etnaviv/drm/etnaviv_drm_winsys.c | 81 +++-------------------
3 files changed, 8 insertions(+), 78 deletions(-)
diff --git a/src/gallium/drivers/etnaviv/etnaviv_screen.c b/src/gallium/drivers/etnaviv/etnaviv_screen.c
index 9aca90642c37..92950c3bfbb5 100644
--- a/src/gallium/drivers/etnaviv/etnaviv_screen.c
+++ b/src/gallium/drivers/etnaviv/etnaviv_screen.c
@@ -806,7 +806,6 @@ etna_screen_create(struct etna_device *dev, struct etna_gpu *gpu,
screen->dev = dev;
screen->gpu = gpu;
screen->ro = renderonly_dup(ro);
- screen->refcnt = 1;
if (!screen->ro) {
DBG("could not create renderonly object");
diff --git a/src/gallium/drivers/etnaviv/etnaviv_screen.h b/src/gallium/drivers/etnaviv/etnaviv_screen.h
index dc57a38dbb80..74b8e98d1695 100644
--- a/src/gallium/drivers/etnaviv/etnaviv_screen.h
+++ b/src/gallium/drivers/etnaviv/etnaviv_screen.h
@@ -30,7 +30,6 @@
#include "etnaviv_internal.h"
-#include "os/os_thread.h"
#include "pipe/p_screen.h"
#include "renderonly/renderonly.h"
#include "util/slab.h"
@@ -59,9 +58,6 @@ enum viv_features_word {
struct etna_screen {
struct pipe_screen base;
- int refcnt;
- void *winsys_priv;
-
struct etna_device *dev;
struct etna_gpu *gpu;
struct etna_pipe *pipe;
diff --git a/src/gallium/winsys/etnaviv/drm/etnaviv_drm_winsys.c b/src/gallium/winsys/etnaviv/drm/etnaviv_drm_winsys.c
index 8e3f7a06a9a0..09b20389810b 100644
--- a/src/gallium/winsys/etnaviv/drm/etnaviv_drm_winsys.c
+++ b/src/gallium/winsys/etnaviv/drm/etnaviv_drm_winsys.c
@@ -28,6 +28,7 @@
#include "util/u_hash_table.h"
#include "util/u_memory.h"
+#include "util/u_screen.h"
#include "etnaviv/etnaviv_screen.h"
#include "etnaviv/hw/common.xml.h"
@@ -67,85 +68,19 @@ screen_create(struct renderonly *ro)
return etna_screen_create(dev, gpu, ro);
}
-static struct util_hash_table *etna_tab = NULL;
-
-static mtx_t etna_screen_mutex = _MTX_INITIALIZER_NP;
-
-static void
-etna_drm_screen_destroy(struct pipe_screen *pscreen)
-{
- struct etna_screen *screen = etna_screen(pscreen);
- boolean destroy;
-
- mtx_lock(&etna_screen_mutex);
- destroy = --screen->refcnt == 0;
- if (destroy) {
- int fd = etna_device_fd(screen->dev);
- util_hash_table_remove(etna_tab, intptr_to_pointer(fd));
- }
- mtx_unlock(&etna_screen_mutex);
-
- if (destroy) {
- pscreen->destroy = screen->winsys_priv;
- pscreen->destroy(pscreen);
- }
-}
-
-static unsigned hash_fd(void *key)
-{
- int fd = pointer_to_intptr(key);
- struct stat stat;
-
- fstat(fd, &stat);
-
- return stat.st_dev ^ stat.st_ino ^ stat.st_rdev;
-}
-
-static int compare_fd(void *key1, void *key2)
-{
- int fd1 = pointer_to_intptr(key1);
- int fd2 = pointer_to_intptr(key2);
- struct stat stat1, stat2;
-
- fstat(fd1, &stat1);
- fstat(fd2, &stat2);
-
- return stat1.st_dev != stat2.st_dev ||
- stat1.st_ino != stat2.st_ino ||
- stat1.st_rdev != stat2.st_rdev;
-}
-
struct pipe_screen *
etna_drm_screen_create_renderonly(struct renderonly *ro)
{
- struct pipe_screen *pscreen = NULL;
+ struct etna_screen *screen;
+ struct pipe_screen *pscreen = pipe_screen_reference(ro->gpu_fd);
- mtx_lock(&etna_screen_mutex);
- if (!etna_tab) {
- etna_tab = util_hash_table_create(hash_fd, compare_fd);
- if (!etna_tab)
- goto unlock;
- }
+ if (pscreen)
+ return pscreen;
- pscreen = util_hash_table_get(etna_tab, intptr_to_pointer(ro->gpu_fd));
- if (pscreen) {
- etna_screen(pscreen)->refcnt++;
- } else {
- pscreen = screen_create(ro);
- if (pscreen) {
- int fd = etna_device_fd(etna_screen(pscreen)->dev);
- util_hash_table_set(etna_tab, intptr_to_pointer(fd), pscreen);
-
- /* Bit of a hack, to avoid circular linkage dependency,
- * ie. pipe driver having to call in to winsys, we
- * override the pipe drivers screen->destroy() */
- etna_screen(pscreen)->winsys_priv = pscreen->destroy;
- pscreen->destroy = etna_drm_screen_destroy;
- }
- }
+ pscreen = screen_create(ro);
- mtx_unlock(&etna_screen_mutex);
+ screen = etna_screen(pscreen);
+ pipe_screen_reference_init(pscreen, etna_device_fd(screen->dev));
return pscreen;
}
--
2.11.0
Rob Herring
2017-08-07 22:58:04 UTC
Reply
Permalink
Raw Message
Use pipe_screen_unreference as it will call pipe_screen->destroy() when
the pipe_screen is no longer referenced.

The pipe_screen referencing is done within create_screen() functions
as drivers (like amdgpu) may have special needs for ref counting.

Signed-off-by: Rob Herring <***@kernel.org>
Cc: Emil Velikov <***@gmail.com>
---
src/gallium/auxiliary/pipe-loader/pipe_loader.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/gallium/auxiliary/pipe-loader/pipe_loader.c b/src/gallium/auxiliary/pipe-loader/pipe_loader.c
index 4ea3dc81a64a..db58e3d908fd 100644
--- a/src/gallium/auxiliary/pipe-loader/pipe_loader.c
+++ b/src/gallium/auxiliary/pipe-loader/pipe_loader.c
@@ -31,6 +31,7 @@
#include "util/u_memory.h"
#include "util/u_string.h"
#include "util/u_dl.h"
+#include "util/u_screen.h"
#include "util/u_thread.h"
#include "util/xmlconfig.h"
#include "util/xmlpool.h"
@@ -75,7 +76,7 @@ pipe_loader_release_dev(struct pipe_loader_device *dev)
{
mtx_lock(&loader_mutex);

- dev->pscreen->destroy(dev->pscreen);
+ pipe_screen_unreference(dev->pscreen);
dev->ops->release(&dev);

mtx_unlock(&loader_mutex);
--
2.11.0
Rob Herring
2017-08-07 22:58:05 UTC
Reply
Permalink
Raw Message
Use the common pipe_screen ref counting and fd hashing functions.
The mutex can be dropped as the pipe loader serializes the
create_screen() and destroy() calls.

Signed-off-by: Rob Herring <***@kernel.org>
Cc: Christian Gmeiner <***@gmail.com>
Cc: Wladimir J. van der Laan <***@gmail.com>
Cc: Lucas Stach <***@pengutronix.de>
---
src/gallium/drivers/etnaviv/etnaviv_screen.c | 1 -
src/gallium/drivers/etnaviv/etnaviv_screen.h | 4 --
.../winsys/etnaviv/drm/etnaviv_drm_winsys.c | 81 +++-------------------
3 files changed, 8 insertions(+), 78 deletions(-)

diff --git a/src/gallium/drivers/etnaviv/etnaviv_screen.c b/src/gallium/drivers/etnaviv/etnaviv_screen.c
index 9aca90642c37..92950c3bfbb5 100644
--- a/src/gallium/drivers/etnaviv/etnaviv_screen.c
+++ b/src/gallium/drivers/etnaviv/etnaviv_screen.c
@@ -806,7 +806,6 @@ etna_screen_create(struct etna_device *dev, struct etna_gpu *gpu,
screen->dev = dev;
screen->gpu = gpu;
screen->ro = renderonly_dup(ro);
- screen->refcnt = 1;

if (!screen->ro) {
DBG("could not create renderonly object");
diff --git a/src/gallium/drivers/etnaviv/etnaviv_screen.h b/src/gallium/drivers/etnaviv/etnaviv_screen.h
index dc57a38dbb80..74b8e98d1695 100644
--- a/src/gallium/drivers/etnaviv/etnaviv_screen.h
+++ b/src/gallium/drivers/etnaviv/etnaviv_screen.h
@@ -30,7 +30,6 @@

#include "etnaviv_internal.h"

-#include "os/os_thread.h"
#include "pipe/p_screen.h"
#include "renderonly/renderonly.h"
#include "util/slab.h"
@@ -59,9 +58,6 @@ enum viv_features_word {
struct etna_screen {
struct pipe_screen base;

- int refcnt;
- void *winsys_priv;
-
struct etna_device *dev;
struct etna_gpu *gpu;
struct etna_pipe *pipe;
diff --git a/src/gallium/winsys/etnaviv/drm/etnaviv_drm_winsys.c b/src/gallium/winsys/etnaviv/drm/etnaviv_drm_winsys.c
index 8e3f7a06a9a0..09b20389810b 100644
--- a/src/gallium/winsys/etnaviv/drm/etnaviv_drm_winsys.c
+++ b/src/gallium/winsys/etnaviv/drm/etnaviv_drm_winsys.c
@@ -28,6 +28,7 @@

#include "util/u_hash_table.h"
#include "util/u_memory.h"
+#include "util/u_screen.h"

#include "etnaviv/etnaviv_screen.h"
#include "etnaviv/hw/common.xml.h"
@@ -67,85 +68,19 @@ screen_create(struct renderonly *ro)
return etna_screen_create(dev, gpu, ro);
}

-static struct util_hash_table *etna_tab = NULL;
-
-static mtx_t etna_screen_mutex = _MTX_INITIALIZER_NP;
-
-static void
-etna_drm_screen_destroy(struct pipe_screen *pscreen)
-{
- struct etna_screen *screen = etna_screen(pscreen);
- boolean destroy;
-
- mtx_lock(&etna_screen_mutex);
- destroy = --screen->refcnt == 0;
- if (destroy) {
- int fd = etna_device_fd(screen->dev);
- util_hash_table_remove(etna_tab, intptr_to_pointer(fd));
- }
- mtx_unlock(&etna_screen_mutex);
-
- if (destroy) {
- pscreen->destroy = screen->winsys_priv;
- pscreen->destroy(pscreen);
- }
-}
-
-static unsigned hash_fd(void *key)
-{
- int fd = pointer_to_intptr(key);
- struct stat stat;
-
- fstat(fd, &stat);
-
- return stat.st_dev ^ stat.st_ino ^ stat.st_rdev;
-}
-
-static int compare_fd(void *key1, void *key2)
-{
- int fd1 = pointer_to_intptr(key1);
- int fd2 = pointer_to_intptr(key2);
- struct stat stat1, stat2;
-
- fstat(fd1, &stat1);
- fstat(fd2, &stat2);
-
- return stat1.st_dev != stat2.st_dev ||
- stat1.st_ino != stat2.st_ino ||
- stat1.st_rdev != stat2.st_rdev;
-}
-
struct pipe_screen *
etna_drm_screen_create_renderonly(struct renderonly *ro)
{
- struct pipe_screen *pscreen = NULL;
+ struct etna_screen *screen;
+ struct pipe_screen *pscreen = pipe_screen_reference(ro->gpu_fd);

- mtx_lock(&etna_screen_mutex);
- if (!etna_tab) {
- etna_tab = util_hash_table_create(hash_fd, compare_fd);
- if (!etna_tab)
- goto unlock;
- }
+ if (pscreen)
+ return pscreen;

- pscreen = util_hash_table_get(etna_tab, intptr_to_pointer(ro->gpu_fd));
- if (pscreen) {
- etna_screen(pscreen)->refcnt++;
- } else {
- pscreen = screen_create(ro);
- if (pscreen) {
- int fd = etna_device_fd(etna_screen(pscreen)->dev);
- util_hash_table_set(etna_tab, intptr_to_pointer(fd), pscreen);
-
- /* Bit of a hack, to avoid circular linkage dependency,
- * ie. pipe driver having to call in to winsys, we
- * override the pipe drivers screen->destroy() */
- etna_screen(pscreen)->winsys_priv = pscreen->destroy;
- pscreen->destroy = etna_drm_screen_destroy;
- }
- }
+ pscreen = screen_create(ro);

-unlock:
- mtx_unlock(&etna_screen_mutex);
+ screen = etna_screen(pscreen);
+ pipe_screen_reference_init(pscreen, etna_device_fd(screen->dev));
return pscreen;
}
--
2.11.0
Rob Herring
2017-08-07 22:58:06 UTC
Reply
Permalink
Raw Message
Use pipe_screen_unreference as it will call pipe_screen->destroy() when
the pipe_screen is no longer referenced.

Signed-off-by: Rob Herring <***@kernel.org>
Cc: Emil Velikov <***@gmail.com>
---
src/gallium/auxiliary/pipe-loader/pipe_loader.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/gallium/auxiliary/pipe-loader/pipe_loader.c b/src/gallium/auxiliary/pipe-loader/pipe_loader.c
index 4ea3dc81a64a..db58e3d908fd 100644
--- a/src/gallium/auxiliary/pipe-loader/pipe_loader.c
+++ b/src/gallium/auxiliary/pipe-loader/pipe_loader.c
@@ -31,6 +31,7 @@
#include "util/u_memory.h"
#include "util/u_string.h"
#include "util/u_dl.h"
+#include "util/u_screen.h"
#include "util/u_thread.h"
#include "util/xmlconfig.h"
#include "util/xmlpool.h"
@@ -75,7 +76,7 @@ pipe_loader_release_dev(struct pipe_loader_device *dev)
{
mtx_lock(&loader_mutex);

- dev->pscreen->destroy(dev->pscreen);
+ pipe_screen_unreference(dev->pscreen);
dev->ops->release(&dev);

mtx_unlock(&loader_mutex);
--
2.11.0
Rob Herring
2017-08-07 22:58:09 UTC
Reply
Permalink
Raw Message
Use the common pipe_screen ref counting and fd hashing functions.
The mutex can be dropped as the pipe loader serializes the
create_screen() and destroy() calls.

Signed-off-by: Rob Herring <***@kernel.org>
Cc: Rob Clark <***@freedesktop.org>
---
src/gallium/drivers/freedreno/freedreno_screen.c | 1 -
src/gallium/drivers/freedreno/freedreno_screen.h | 10 ---
.../winsys/freedreno/drm/freedreno_drm_winsys.c | 98 ++--------------------
3 files changed, 9 insertions(+), 100 deletions(-)

diff --git a/src/gallium/drivers/freedreno/freedreno_screen.c b/src/gallium/drivers/freedreno/freedreno_screen.c
index b26f67e4e29e..ab3815205ea2 100644
--- a/src/gallium/drivers/freedreno/freedreno_screen.c
+++ b/src/gallium/drivers/freedreno/freedreno_screen.c
@@ -745,7 +745,6 @@ fd_screen_create(struct fd_device *dev)
pscreen = &screen->base;

screen->dev = dev;
- screen->refcnt = 1;

// maybe this should be in context?
screen->pipe = fd_pipe_new(screen->dev, FD_PIPE_3D);
diff --git a/src/gallium/drivers/freedreno/freedreno_screen.h b/src/gallium/drivers/freedreno/freedreno_screen.h
index c5018da4bc59..586119d1d4a9 100644
--- a/src/gallium/drivers/freedreno/freedreno_screen.h
+++ b/src/gallium/drivers/freedreno/freedreno_screen.h
@@ -47,16 +47,6 @@ struct fd_screen {

mtx_t lock;

- /* it would be tempting to use pipe_reference here, but that
- * really doesn't work well if it isn't the first member of
- * the struct, so not quite so awesome to be adding refcnting
- * further down the inheritance hierarchy:
- */
- int refcnt;
-
- /* place for winsys to stash it's own stuff: */
- void *winsys_priv;
-
struct slab_parent_pool transfer_pool;

uint32_t gmemsize_bytes;
diff --git a/src/gallium/winsys/freedreno/drm/freedreno_drm_winsys.c b/src/gallium/winsys/freedreno/drm/freedreno_drm_winsys.c
index c1ea22a06482..7d61ec9421e6 100644
--- a/src/gallium/winsys/freedreno/drm/freedreno_drm_winsys.c
+++ b/src/gallium/winsys/freedreno/drm/freedreno_drm_winsys.c
@@ -26,102 +26,22 @@
* Rob Clark <***@freedesktop.org>
*/

-#include <sys/stat.h>
-
-#include "pipe/p_context.h"
-#include "pipe/p_state.h"
-#include "util/u_format.h"
-#include "util/u_memory.h"
-#include "util/u_inlines.h"
-#include "util/u_hash_table.h"
-#include "os/os_thread.h"
+#include "util/u_screen.h"

#include "freedreno_drm_public.h"

#include "freedreno/freedreno_screen.h"

-static struct util_hash_table *fd_tab = NULL;
-
-static mtx_t fd_screen_mutex = _MTX_INITIALIZER_NP;
-
-static void
-fd_drm_screen_destroy(struct pipe_screen *pscreen)
-{
- struct fd_screen *screen = fd_screen(pscreen);
- boolean destroy;
-
- mtx_lock(&fd_screen_mutex);
- destroy = --screen->refcnt == 0;
- if (destroy) {
- int fd = fd_device_fd(screen->dev);
- util_hash_table_remove(fd_tab, intptr_to_pointer(fd));
- }
- mtx_unlock(&fd_screen_mutex);
-
- if (destroy) {
- pscreen->destroy = screen->winsys_priv;
- pscreen->destroy(pscreen);
- }
-}
-
-static unsigned hash_fd(void *key)
-{
- int fd = pointer_to_intptr(key);
- struct stat stat;
- fstat(fd, &stat);
-
- return stat.st_dev ^ stat.st_ino ^ stat.st_rdev;
-}
-
-static int compare_fd(void *key1, void *key2)
-{
- int fd1 = pointer_to_intptr(key1);
- int fd2 = pointer_to_intptr(key2);
- struct stat stat1, stat2;
- fstat(fd1, &stat1);
- fstat(fd2, &stat2);
-
- return stat1.st_dev != stat2.st_dev ||
- stat1.st_ino != stat2.st_ino ||
- stat1.st_rdev != stat2.st_rdev;
-}
-
struct pipe_screen *
fd_drm_screen_create(int fd)
{
- struct pipe_screen *pscreen = NULL;
-
- mtx_lock(&fd_screen_mutex);
- if (!fd_tab) {
- fd_tab = util_hash_table_create(hash_fd, compare_fd);
- if (!fd_tab)
- goto unlock;
- }
-
- pscreen = util_hash_table_get(fd_tab, intptr_to_pointer(fd));
- if (pscreen) {
- fd_screen(pscreen)->refcnt++;
- } else {
- struct fd_device *dev = fd_device_new_dup(fd);
- if (!dev)
- goto unlock;
-
- pscreen = fd_screen_create(dev);
- if (pscreen) {
- int fd = fd_device_fd(dev);
-
- util_hash_table_set(fd_tab, intptr_to_pointer(fd), pscreen);
-
- /* Bit of a hack, to avoid circular linkage dependency,
- * ie. pipe driver having to call in to winsys, we
- * override the pipe drivers screen->destroy():
- */
- fd_screen(pscreen)->winsys_priv = pscreen->destroy;
- pscreen->destroy = fd_drm_screen_destroy;
- }
- }
-
-unlock:
- mtx_unlock(&fd_screen_mutex);
+ int dupfd;
+ struct pipe_screen *pscreen = pipe_screen_reference(fd);
+ if (pscreen)
+ return pscreen;
+
+ dupfd = dup(fd);
+ pscreen = fd_screen_create(fd_device_new(dupfd));
+ pipe_screen_reference_init(pscreen, dupfd);
return pscreen;
}
--
2.11.0
Rob Herring
2017-08-07 22:58:10 UTC
Reply
Permalink
Raw Message
Use the common pipe_screen ref counting and fd hashing functions.
The mutex can be dropped as the pipe loader serializes the
create_screen() and destroy() calls.

Signed-off-by: Rob Herring <***@kernel.org>
Cc: Alexandre Courbot <***@nvidia.com>
---
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 -
.../winsys/nouveau/drm/nouveau_drm_winsys.c | 69 ++--------------------
6 files changed, 5 insertions(+), 83 deletions(-)

diff --git a/src/gallium/drivers/nouveau/nouveau_screen.c b/src/gallium/drivers/nouveau/nouveau_screen.c
index 13b76d768165..03524ef2e867 100644
--- a/src/gallium/drivers/nouveau/nouveau_screen.c
+++ b/src/gallium/drivers/nouveau/nouveau_screen.c
@@ -185,12 +185,6 @@ nouveau_screen_init(struct nouveau_screen *screen, struct nouveau_device *dev)
screen->drm = nouveau_drm(&dev->object);
screen->device = dev;

- /*
- * this is initialized to 1 in nouveau_drm_screen_create after screen
- * is fully constructed and added to the global screen list.
- */
- screen->refcount = -1;
-
if (dev->chipset < 0xc0) {
data = &nv04_data;
size = sizeof(nv04_data);
diff --git a/src/gallium/drivers/nouveau/nouveau_screen.h b/src/gallium/drivers/nouveau/nouveau_screen.h
index e4fbae99ca44..de4b66b4e8ef 100644
--- a/src/gallium/drivers/nouveau/nouveau_screen.h
+++ b/src/gallium/drivers/nouveau/nouveau_screen.h
@@ -24,8 +24,6 @@ struct nouveau_screen {
struct nouveau_client *client;
struct nouveau_pushbuf *pushbuf;

- int refcount;
-
unsigned vidmem_bindings; /* PIPE_BIND_* where VRAM placement is desired */
unsigned sysmem_bindings; /* PIPE_BIND_* where GART placement is desired */
unsigned lowmem_bindings; /* PIPE_BIND_* that require an address < 4 GiB */
@@ -122,8 +120,6 @@ nouveau_screen(struct pipe_screen *pscreen)
return (struct nouveau_screen *)pscreen;
}

-bool nouveau_drm_screen_unref(struct nouveau_screen *screen);
-
bool
nouveau_screen_bo_get_handle(struct pipe_screen *pscreen,
struct nouveau_bo *bo,
diff --git a/src/gallium/drivers/nouveau/nv30/nv30_screen.c b/src/gallium/drivers/nouveau/nv30/nv30_screen.c
index 72f886c9114e..14e9e1b8e4a8 100644
--- a/src/gallium/drivers/nouveau/nv30/nv30_screen.c
+++ b/src/gallium/drivers/nouveau/nv30/nv30_screen.c
@@ -431,9 +431,6 @@ nv30_screen_destroy(struct pipe_screen *pscreen)
{
struct nv30_screen *screen = nv30_screen(pscreen);

- if (!nouveau_drm_screen_unref(&screen->base))
- return;
-
if (screen->base.fence.current) {
struct nouveau_fence *current = NULL;

diff --git a/src/gallium/drivers/nouveau/nv50/nv50_screen.c b/src/gallium/drivers/nouveau/nv50/nv50_screen.c
index 0f25cd5fedd7..887de2c7531d 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_screen.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_screen.c
@@ -455,9 +455,6 @@ nv50_screen_destroy(struct pipe_screen *pscreen)
{
struct nv50_screen *screen = nv50_screen(pscreen);

- if (!nouveau_drm_screen_unref(&screen->base))
- return;
-
if (screen->base.fence.current) {
struct nouveau_fence *current = NULL;

diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
index f64ac2625def..4405c9930b09 100644
--- a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
@@ -527,9 +527,6 @@ nvc0_screen_destroy(struct pipe_screen *pscreen)
{
struct nvc0_screen *screen = nvc0_screen(pscreen);

- if (!nouveau_drm_screen_unref(&screen->base))
- return;
-
if (screen->base.fence.current) {
struct nouveau_fence *current = NULL;

diff --git a/src/gallium/winsys/nouveau/drm/nouveau_drm_winsys.c b/src/gallium/winsys/nouveau/drm/nouveau_drm_winsys.c
index 4ca2d35ea33a..4730d9c7b946 100644
--- a/src/gallium/winsys/nouveau/drm/nouveau_drm_winsys.c
+++ b/src/gallium/winsys/nouveau/drm/nouveau_drm_winsys.c
@@ -1,13 +1,10 @@
-#include <sys/stat.h>
#include <unistd.h>
#include <fcntl.h>
#include "pipe/p_context.h"
#include "pipe/p_state.h"
#include "util/u_format.h"
#include "util/u_memory.h"
-#include "util/u_inlines.h"
-#include "util/u_hash_table.h"
-#include "os/os_thread.h"
+#include "util/u_screen.h"

#include "nouveau_drm_public.h"

@@ -17,47 +14,6 @@
#include <nvif/class.h>
#include <nvif/cl0080.h>

-static struct util_hash_table *fd_tab = NULL;
-
-static mtx_t nouveau_screen_mutex = _MTX_INITIALIZER_NP;
-
-bool nouveau_drm_screen_unref(struct nouveau_screen *screen)
-{
- int ret;
- if (screen->refcount == -1)
- return true;
-
- mtx_lock(&nouveau_screen_mutex);
- ret = --screen->refcount;
- assert(ret >= 0);
- if (ret == 0)
- util_hash_table_remove(fd_tab, intptr_to_pointer(screen->drm->fd));
- mtx_unlock(&nouveau_screen_mutex);
- return ret == 0;
-}
-
-static unsigned hash_fd(void *key)
-{
- int fd = pointer_to_intptr(key);
- struct stat stat;
- fstat(fd, &stat);
-
- return stat.st_dev ^ stat.st_ino ^ stat.st_rdev;
-}
-
-static int compare_fd(void *key1, void *key2)
-{
- int fd1 = pointer_to_intptr(key1);
- int fd2 = pointer_to_intptr(key2);
- struct stat stat1, stat2;
- fstat(fd1, &stat1);
- fstat(fd2, &stat2);
-
- return stat1.st_dev != stat2.st_dev ||
- stat1.st_ino != stat2.st_ino ||
- stat1.st_rdev != stat2.st_rdev;
-}
-
PUBLIC struct pipe_screen *
nouveau_drm_screen_create(int fd)
{
@@ -65,23 +21,11 @@ nouveau_drm_screen_create(int fd)
struct nouveau_device *dev = NULL;
struct nouveau_screen *(*init)(struct nouveau_device *);
struct nouveau_screen *screen = NULL;
+ struct pipe_screen *pscreen = pipe_screen_reference(fd);
int ret, dupfd;

- mtx_lock(&nouveau_screen_mutex);
- if (!fd_tab) {
- fd_tab = util_hash_table_create(hash_fd, compare_fd);
- if (!fd_tab) {
- mtx_unlock(&nouveau_screen_mutex);
- return NULL;
- }
- }
-
- screen = util_hash_table_get(fd_tab, intptr_to_pointer(fd));
- if (screen) {
- screen->refcount++;
- mtx_unlock(&nouveau_screen_mutex);
- return &screen->base;
- }
+ if (pscreen)
+ return pscreen;

/* Since the screen re-use is based on the device node and not the fd,
* create a copy of the fd to be owned by the device. Otherwise a
@@ -141,9 +85,7 @@ nouveau_drm_screen_create(int fd)
* closed by its owner. The hash key needs to live at least as long as
* the screen.
*/
- util_hash_table_set(fd_tab, intptr_to_pointer(dupfd), screen);
- screen->refcount = 1;
- mtx_unlock(&nouveau_screen_mutex);
+ pipe_screen_reference_init(&screen->base, dupfd);
return &screen->base;

err:
@@ -154,6 +96,5 @@ err:
nouveau_drm_del(&drm);
close(dupfd);
}
- mtx_unlock(&nouveau_screen_mutex);
return NULL;
}
--
2.11.0
Rob Herring
2017-08-07 22:58:11 UTC
Reply
Permalink
Raw Message
Use the common pipe_screen ref counting and fd hashing functions.
The mutex can be dropped as the pipe loader serializes the
create_screen() and destroy() calls.

Signed-off-by: Rob Herring <***@kernel.org>
Cc: Alexandre Courbot <***@nvidia.com>
---
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 -
.../winsys/nouveau/drm/nouveau_drm_winsys.c | 69 ++--------------------
6 files changed, 5 insertions(+), 83 deletions(-)

diff --git a/src/gallium/drivers/nouveau/nouveau_screen.c b/src/gallium/drivers/nouveau/nouveau_screen.c
index 13b76d768165..03524ef2e867 100644
--- a/src/gallium/drivers/nouveau/nouveau_screen.c
+++ b/src/gallium/drivers/nouveau/nouveau_screen.c
@@ -185,12 +185,6 @@ nouveau_screen_init(struct nouveau_screen *screen, struct nouveau_device *dev)
screen->drm = nouveau_drm(&dev->object);
screen->device = dev;

- /*
- * this is initialized to 1 in nouveau_drm_screen_create after screen
- * is fully constructed and added to the global screen list.
- */
- screen->refcount = -1;
-
if (dev->chipset < 0xc0) {
data = &nv04_data;
size = sizeof(nv04_data);
diff --git a/src/gallium/drivers/nouveau/nouveau_screen.h b/src/gallium/drivers/nouveau/nouveau_screen.h
index e4fbae99ca44..de4b66b4e8ef 100644
--- a/src/gallium/drivers/nouveau/nouveau_screen.h
+++ b/src/gallium/drivers/nouveau/nouveau_screen.h
@@ -24,8 +24,6 @@ struct nouveau_screen {
struct nouveau_client *client;
struct nouveau_pushbuf *pushbuf;

- int refcount;
-
unsigned vidmem_bindings; /* PIPE_BIND_* where VRAM placement is desired */
unsigned sysmem_bindings; /* PIPE_BIND_* where GART placement is desired */
unsigned lowmem_bindings; /* PIPE_BIND_* that require an address < 4 GiB */
@@ -122,8 +120,6 @@ nouveau_screen(struct pipe_screen *pscreen)
return (struct nouveau_screen *)pscreen;
}

-bool nouveau_drm_screen_unref(struct nouveau_screen *screen);
-
bool
nouveau_screen_bo_get_handle(struct pipe_screen *pscreen,
struct nouveau_bo *bo,
diff --git a/src/gallium/drivers/nouveau/nv30/nv30_screen.c b/src/gallium/drivers/nouveau/nv30/nv30_screen.c
index 72f886c9114e..14e9e1b8e4a8 100644
--- a/src/gallium/drivers/nouveau/nv30/nv30_screen.c
+++ b/src/gallium/drivers/nouveau/nv30/nv30_screen.c
@@ -431,9 +431,6 @@ nv30_screen_destroy(struct pipe_screen *pscreen)
{
struct nv30_screen *screen = nv30_screen(pscreen);

- if (!nouveau_drm_screen_unref(&screen->base))
- return;
-
if (screen->base.fence.current) {
struct nouveau_fence *current = NULL;

diff --git a/src/gallium/drivers/nouveau/nv50/nv50_screen.c b/src/gallium/drivers/nouveau/nv50/nv50_screen.c
index 0f25cd5fedd7..887de2c7531d 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_screen.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_screen.c
@@ -455,9 +455,6 @@ nv50_screen_destroy(struct pipe_screen *pscreen)
{
struct nv50_screen *screen = nv50_screen(pscreen);

- if (!nouveau_drm_screen_unref(&screen->base))
- return;
-
if (screen->base.fence.current) {
struct nouveau_fence *current = NULL;

diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
index f64ac2625def..4405c9930b09 100644
--- a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
@@ -527,9 +527,6 @@ nvc0_screen_destroy(struct pipe_screen *pscreen)
{
struct nvc0_screen *screen = nvc0_screen(pscreen);

- if (!nouveau_drm_screen_unref(&screen->base))
- return;
-
if (screen->base.fence.current) {
struct nouveau_fence *current = NULL;

diff --git a/src/gallium/winsys/nouveau/drm/nouveau_drm_winsys.c b/src/gallium/winsys/nouveau/drm/nouveau_drm_winsys.c
index 4ca2d35ea33a..4730d9c7b946 100644
--- a/src/gallium/winsys/nouveau/drm/nouveau_drm_winsys.c
+++ b/src/gallium/winsys/nouveau/drm/nouveau_drm_winsys.c
@@ -1,13 +1,10 @@
-#include <sys/stat.h>
#include <unistd.h>
#include <fcntl.h>
#include "pipe/p_context.h"
#include "pipe/p_state.h"
#include "util/u_format.h"
#include "util/u_memory.h"
-#include "util/u_inlines.h"
-#include "util/u_hash_table.h"
-#include "os/os_thread.h"
+#include "util/u_screen.h"

#include "nouveau_drm_public.h"

@@ -17,47 +14,6 @@
#include <nvif/class.h>
#include <nvif/cl0080.h>

-static struct util_hash_table *fd_tab = NULL;
-
-static mtx_t nouveau_screen_mutex = _MTX_INITIALIZER_NP;
-
-bool nouveau_drm_screen_unref(struct nouveau_screen *screen)
-{
- int ret;
- if (screen->refcount == -1)
- return true;
-
- mtx_lock(&nouveau_screen_mutex);
- ret = --screen->refcount;
- assert(ret >= 0);
- if (ret == 0)
- util_hash_table_remove(fd_tab, intptr_to_pointer(screen->drm->fd));
- mtx_unlock(&nouveau_screen_mutex);
- return ret == 0;
-}
-
-static unsigned hash_fd(void *key)
-{
- int fd = pointer_to_intptr(key);
- struct stat stat;
- fstat(fd, &stat);
-
- return stat.st_dev ^ stat.st_ino ^ stat.st_rdev;
-}
-
-static int compare_fd(void *key1, void *key2)
-{
- int fd1 = pointer_to_intptr(key1);
- int fd2 = pointer_to_intptr(key2);
- struct stat stat1, stat2;
- fstat(fd1, &stat1);
- fstat(fd2, &stat2);
-
- return stat1.st_dev != stat2.st_dev ||
- stat1.st_ino != stat2.st_ino ||
- stat1.st_rdev != stat2.st_rdev;
-}
-
PUBLIC struct pipe_screen *
nouveau_drm_screen_create(int fd)
{
@@ -65,23 +21,11 @@ nouveau_drm_screen_create(int fd)
struct nouveau_device *dev = NULL;
struct nouveau_screen *(*init)(struct nouveau_device *);
struct nouveau_screen *screen = NULL;
+ struct pipe_screen *pscreen = pipe_screen_reference(fd);
int ret, dupfd;

- mtx_lock(&nouveau_screen_mutex);
- if (!fd_tab) {
- fd_tab = util_hash_table_create(hash_fd, compare_fd);
- if (!fd_tab) {
- mtx_unlock(&nouveau_screen_mutex);
- return NULL;
- }
- }
-
- screen = util_hash_table_get(fd_tab, intptr_to_pointer(fd));
- if (screen) {
- screen->refcount++;
- mtx_unlock(&nouveau_screen_mutex);
- return &screen->base;
- }
+ if (pscreen)
+ return pscreen;

/* Since the screen re-use is based on the device node and not the fd,
* create a copy of the fd to be owned by the device. Otherwise a
@@ -141,9 +85,7 @@ nouveau_drm_screen_create(int fd)
* closed by its owner. The hash key needs to live at least as long as
* the screen.
*/
- util_hash_table_set(fd_tab, intptr_to_pointer(dupfd), screen);
- screen->refcount = 1;
- mtx_unlock(&nouveau_screen_mutex);
+ pipe_screen_reference_init(&screen->base, dupfd);
return &screen->base;

err:
@@ -154,6 +96,5 @@ err:
nouveau_drm_del(&drm);
close(dupfd);
}
- mtx_unlock(&nouveau_screen_mutex);
return NULL;
}
--
2.11.0
Rob Herring
2017-08-07 22:58:08 UTC
Reply
Permalink
Raw Message
Use the common pipe_screen ref counting and fd hashing functions.
The mutex can be dropped as the pipe loader serializes the
create_screen() and destroy() calls.

Signed-off-by: Rob Herring <***@kernel.org>
Cc: Rob Clark <***@freedesktop.org>
---
src/gallium/drivers/freedreno/freedreno_screen.c | 1 -
src/gallium/drivers/freedreno/freedreno_screen.h | 10 ---
.../winsys/freedreno/drm/freedreno_drm_winsys.c | 98 ++--------------------
3 files changed, 9 insertions(+), 100 deletions(-)

diff --git a/src/gallium/drivers/freedreno/freedreno_screen.c b/src/gallium/drivers/freedreno/freedreno_screen.c
index b26f67e4e29e..ab3815205ea2 100644
--- a/src/gallium/drivers/freedreno/freedreno_screen.c
+++ b/src/gallium/drivers/freedreno/freedreno_screen.c
@@ -745,7 +745,6 @@ fd_screen_create(struct fd_device *dev)
pscreen = &screen->base;

screen->dev = dev;
- screen->refcnt = 1;

// maybe this should be in context?
screen->pipe = fd_pipe_new(screen->dev, FD_PIPE_3D);
diff --git a/src/gallium/drivers/freedreno/freedreno_screen.h b/src/gallium/drivers/freedreno/freedreno_screen.h
index c5018da4bc59..586119d1d4a9 100644
--- a/src/gallium/drivers/freedreno/freedreno_screen.h
+++ b/src/gallium/drivers/freedreno/freedreno_screen.h
@@ -47,16 +47,6 @@ struct fd_screen {

mtx_t lock;

- /* it would be tempting to use pipe_reference here, but that
- * really doesn't work well if it isn't the first member of
- * the struct, so not quite so awesome to be adding refcnting
- * further down the inheritance hierarchy:
- */
- int refcnt;
-
- /* place for winsys to stash it's own stuff: */
- void *winsys_priv;
-
struct slab_parent_pool transfer_pool;

uint32_t gmemsize_bytes;
diff --git a/src/gallium/winsys/freedreno/drm/freedreno_drm_winsys.c b/src/gallium/winsys/freedreno/drm/freedreno_drm_winsys.c
index c1ea22a06482..7d61ec9421e6 100644
--- a/src/gallium/winsys/freedreno/drm/freedreno_drm_winsys.c
+++ b/src/gallium/winsys/freedreno/drm/freedreno_drm_winsys.c
@@ -26,102 +26,22 @@
* Rob Clark <***@freedesktop.org>
*/

-#include <sys/stat.h>
-
-#include "pipe/p_context.h"
-#include "pipe/p_state.h"
-#include "util/u_format.h"
-#include "util/u_memory.h"
-#include "util/u_inlines.h"
-#include "util/u_hash_table.h"
-#include "os/os_thread.h"
+#include "util/u_screen.h"

#include "freedreno_drm_public.h"

#include "freedreno/freedreno_screen.h"

-static struct util_hash_table *fd_tab = NULL;
-
-static mtx_t fd_screen_mutex = _MTX_INITIALIZER_NP;
-
-static void
-fd_drm_screen_destroy(struct pipe_screen *pscreen)
-{
- struct fd_screen *screen = fd_screen(pscreen);
- boolean destroy;
-
- mtx_lock(&fd_screen_mutex);
- destroy = --screen->refcnt == 0;
- if (destroy) {
- int fd = fd_device_fd(screen->dev);
- util_hash_table_remove(fd_tab, intptr_to_pointer(fd));
- }
- mtx_unlock(&fd_screen_mutex);
-
- if (destroy) {
- pscreen->destroy = screen->winsys_priv;
- pscreen->destroy(pscreen);
- }
-}
-
-static unsigned hash_fd(void *key)
-{
- int fd = pointer_to_intptr(key);
- struct stat stat;
- fstat(fd, &stat);
-
- return stat.st_dev ^ stat.st_ino ^ stat.st_rdev;
-}
-
-static int compare_fd(void *key1, void *key2)
-{
- int fd1 = pointer_to_intptr(key1);
- int fd2 = pointer_to_intptr(key2);
- struct stat stat1, stat2;
- fstat(fd1, &stat1);
- fstat(fd2, &stat2);
-
- return stat1.st_dev != stat2.st_dev ||
- stat1.st_ino != stat2.st_ino ||
- stat1.st_rdev != stat2.st_rdev;
-}
-
struct pipe_screen *
fd_drm_screen_create(int fd)
{
- struct pipe_screen *pscreen = NULL;
-
- mtx_lock(&fd_screen_mutex);
- if (!fd_tab) {
- fd_tab = util_hash_table_create(hash_fd, compare_fd);
- if (!fd_tab)
- goto unlock;
- }
-
- pscreen = util_hash_table_get(fd_tab, intptr_to_pointer(fd));
- if (pscreen) {
- fd_screen(pscreen)->refcnt++;
- } else {
- struct fd_device *dev = fd_device_new_dup(fd);
- if (!dev)
- goto unlock;
-
- pscreen = fd_screen_create(dev);
- if (pscreen) {
- int fd = fd_device_fd(dev);
-
- util_hash_table_set(fd_tab, intptr_to_pointer(fd), pscreen);
-
- /* Bit of a hack, to avoid circular linkage dependency,
- * ie. pipe driver having to call in to winsys, we
- * override the pipe drivers screen->destroy():
- */
- fd_screen(pscreen)->winsys_priv = pscreen->destroy;
- pscreen->destroy = fd_drm_screen_destroy;
- }
- }
-
-unlock:
- mtx_unlock(&fd_screen_mutex);
+ int dupfd;
+ struct pipe_screen *pscreen = pipe_screen_reference(fd);
+ if (pscreen)
+ return pscreen;
+
+ dupfd = dup(fd);
+ pscreen = fd_screen_create(fd_device_new(dupfd));
+ pipe_screen_reference_init(pscreen, dupfd);
return pscreen;
}
--
2.11.0
Rob Herring
2017-08-07 22:58:13 UTC
Reply
Permalink
Raw Message
Use the common pipe_screen ref counting.

radeon uses the common pipe_screen_{un}reference functions, but amdgpu
is unique in its hashing the dev pointer rather than the fd, so the
common fd hashing cannot be used. However, the same reference counting
can be used instead of the private one. The mutex can be dropped as the
pipe loader serializes the create_screen() and destroy() calls.

Signed-off-by: Rob Herring <***@kernel.org>
Cc: "Marek Olšák" <***@amd.com>
Cc: Ilia Mirkin <***@alum.mit.edu>
---
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/include/pipe/p_screen.h | 2 +
src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c | 44 ++-----------
src/gallium/winsys/amdgpu/drm/amdgpu_winsys.h | 1 -
src/gallium/winsys/radeon/drm/radeon_drm_winsys.c | 79 ++---------------------
src/gallium/winsys/radeon/drm/radeon_drm_winsys.h | 1 -
9 files changed, 13 insertions(+), 134 deletions(-)

diff --git a/src/gallium/drivers/r300/r300_screen.c b/src/gallium/drivers/r300/r300_screen.c
index ff68ffd7bc65..6991e8396638 100644
--- a/src/gallium/drivers/r300/r300_screen.c
+++ b/src/gallium/drivers/r300/r300_screen.c
@@ -696,9 +696,6 @@ static void r300_destroy_screen(struct pipe_screen* pscreen)
struct r300_screen* r300screen = r300_screen(pscreen);
struct radeon_winsys *rws = radeon_winsys(pscreen);

- if (rws && !rws->unref(rws))
- return;
-
mtx_destroy(&r300screen->cmask_mutex);
slab_destroy_parent(&r300screen->pool_transfers);

diff --git a/src/gallium/drivers/r600/r600_pipe.c b/src/gallium/drivers/r600/r600_pipe.c
index 023f1b4bd14f..c0f7312e956d 100644
--- a/src/gallium/drivers/r600/r600_pipe.c
+++ b/src/gallium/drivers/r600/r600_pipe.c
@@ -612,12 +612,6 @@ static void r600_destroy_screen(struct pipe_screen* pscreen)
{
struct r600_screen *rscreen = (struct r600_screen *)pscreen;

- if (!rscreen)
- return;
-
- if (!rscreen->b.ws->unref(rscreen->b.ws))
- return;
-
if (rscreen->global_pool) {
compute_memory_pool_delete(rscreen->global_pool);
}
diff --git a/src/gallium/drivers/radeon/radeon_winsys.h b/src/gallium/drivers/radeon/radeon_winsys.h
index 351edcd76f98..6ccd2ddbb0b2 100644
--- a/src/gallium/drivers/radeon/radeon_winsys.h
+++ b/src/gallium/drivers/radeon/radeon_winsys.h
@@ -230,14 +230,6 @@ struct radeon_winsys {
struct pipe_screen *screen;

/**
- * Decrement the winsys reference count.
- *
- * \param ws The winsys this function is called for.
- * \return True if the winsys and screen should be destroyed.
- */
- bool (*unref)(struct radeon_winsys *ws);
-
- /**
* Destroy this winsys.
*
* \param ws The winsys this function is called from.
diff --git a/src/gallium/drivers/radeonsi/si_pipe.c b/src/gallium/drivers/radeonsi/si_pipe.c
index 395853c7d9f3..881ed0c58339 100644
--- a/src/gallium/drivers/radeonsi/si_pipe.c
+++ b/src/gallium/drivers/radeonsi/si_pipe.c
@@ -843,9 +843,6 @@ static void si_destroy_screen(struct pipe_screen* pscreen)
};
unsigned i;

- if (!sscreen->b.ws->unref(sscreen->b.ws))
- return;
-
util_queue_destroy(&sscreen->shader_compiler_queue);
util_queue_destroy(&sscreen->shader_compiler_queue_low_priority);

diff --git a/src/gallium/include/pipe/p_screen.h b/src/gallium/include/pipe/p_screen.h
index dbb14ef882c0..8f866f83d763 100644
--- a/src/gallium/include/pipe/p_screen.h
+++ b/src/gallium/include/pipe/p_screen.h
@@ -71,6 +71,8 @@ struct driOptionCache;
struct pipe_screen {
struct pipe_reference reference;
int fd;
+ void *ws;
+
void (*destroy)( struct pipe_screen * );

const char *(*get_name)( struct pipe_screen * );
diff --git a/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c b/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c
index 26c7dacb9df4..258872388ce3 100644
--- a/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c
+++ b/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c
@@ -48,7 +48,6 @@
#endif

static struct util_hash_table *dev_tab = NULL;
-static mtx_t dev_tab_mutex = _MTX_INITIALIZER_NP;

/* Helper function to do the ioctls needed for setup and init. */
static bool do_winsys_init(struct amdgpu_winsys *ws, int fd)
@@ -97,6 +96,7 @@ static void amdgpu_winsys_destroy(struct radeon_winsys *rws)
pb_cache_deinit(&ws->bo_cache);
mtx_destroy(&ws->global_bo_list_lock);
do_winsys_deinit(ws);
+ util_hash_table_remove(dev_tab, ws->dev);
FREE(rws);
}

@@ -203,26 +203,6 @@ static int compare_dev(void *key1, void *key2)
return key1 != key2;
}

-static bool amdgpu_winsys_unref(struct radeon_winsys *rws)
-{
- struct amdgpu_winsys *ws = (struct amdgpu_winsys*)rws;
- bool destroy;
-
- /* When the reference counter drops to zero, remove the device pointer
- * from the table.
- * This must happen while the mutex is locked, so that
- * amdgpu_winsys_create in another thread doesn't get the winsys
- * from the table when the counter drops to 0. */
- mtx_lock(&dev_tab_mutex);
-
- destroy = pipe_reference(&ws->reference, NULL);
- if (destroy && dev_tab)
- util_hash_table_remove(dev_tab, ws->dev);
-
- mtx_unlock(&dev_tab_mutex);
- return destroy;
-}
-
static const char* amdgpu_get_chip_name(struct radeon_winsys *ws)
{
amdgpu_device_handle dev = ((struct amdgpu_winsys *)ws)->dev;
@@ -247,7 +227,6 @@ amdgpu_winsys_create(int fd, const struct pipe_screen_config *config,
drmFreeVersion(version);

/* Look up the winsys from the dev table. */
- mtx_lock(&dev_tab_mutex);
if (!dev_tab)
dev_tab = util_hash_table_create(hash_dev, compare_dev);

@@ -255,7 +234,6 @@ amdgpu_winsys_create(int fd, const struct pipe_screen_config *config,
* for the same fd. */
r = amdgpu_device_initialize(fd, &drm_major, &drm_minor, &dev);
if (r) {
- mtx_unlock(&dev_tab_mutex);
fprintf(stderr, "amdgpu: amdgpu_device_initialize failed.\n");
return NULL;
}
@@ -263,15 +241,14 @@ amdgpu_winsys_create(int fd, const struct pipe_screen_config *config,
/* Lookup a winsys if we have already created one for this device. */
ws = util_hash_table_get(dev_tab, dev);
if (ws) {
- pipe_reference(NULL, &ws->reference);
- mtx_unlock(&dev_tab_mutex);
+ pipe_reference(NULL, &ws->base.screen->reference);
return &ws->base;
}

/* Create a new winsys. */
ws = CALLOC_STRUCT(amdgpu_winsys);
if (!ws)
- goto fail;
+ return NULL;

ws->dev = dev;
ws->info.drm_major = drm_major;
@@ -296,11 +273,7 @@ amdgpu_winsys_create(int fd, const struct pipe_screen_config *config,

ws->info.min_alloc_size = 1 << AMDGPU_SLAB_MIN_SIZE_LOG2;

- /* init reference */
- pipe_reference_init(&ws->reference, 1);
-
/* Set functions. */
- ws->base.unref = amdgpu_winsys_unref;
ws->base.destroy = amdgpu_winsys_destroy;
ws->base.query_info = amdgpu_winsys_query_info;
ws->base.cs_request_feature = amdgpu_cs_request_feature;
@@ -319,7 +292,6 @@ amdgpu_winsys_create(int fd, const struct pipe_screen_config *config,
if (!util_queue_init(&ws->cs_queue, "amdgpu_cs", 8, 1,
UTIL_QUEUE_INIT_RESIZE_IF_FULL)) {
amdgpu_winsys_destroy(&ws->base);
- mtx_unlock(&dev_tab_mutex);
return NULL;
}

@@ -331,16 +303,12 @@ amdgpu_winsys_create(int fd, const struct pipe_screen_config *config,
ws->base.screen = screen_create(&ws->base, config);
if (!ws->base.screen) {
amdgpu_winsys_destroy(&ws->base);
- mtx_unlock(&dev_tab_mutex);
return NULL;
}

util_hash_table_set(dev_tab, dev, ws);
-
- /* We must unlock the mutex once the winsys is fully initialized, so that
- * other threads attempting to create the winsys from the same fd will
- * get a fully initialized winsys and not just half-way initialized. */
- mtx_unlock(&dev_tab_mutex);
+ ws->base.screen->fd = -1;
+ pipe_reference_init(&ws->base.screen->reference, 1);

return &ws->base;

@@ -349,7 +317,5 @@ fail_cache:
do_winsys_deinit(ws);
fail_alloc:
FREE(ws);
-fail:
- mtx_unlock(&dev_tab_mutex);
return NULL;
}
diff --git a/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.h b/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.h
index 7aca612f4523..52f00b33c1a6 100644
--- a/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.h
+++ b/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.h
@@ -47,7 +47,6 @@ struct amdgpu_cs;

struct amdgpu_winsys {
struct radeon_winsys base;
- struct pipe_reference reference;
struct pb_cache bo_cache;
struct pb_slabs bo_slabs;

diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
index 542e5494bebc..d27428598ed2 100644
--- a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
+++ b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
@@ -37,17 +37,15 @@

#include "util/u_memory.h"
#include "util/u_hash_table.h"
+#include "util/u_screen.h"

#include <xf86drm.h>
#include <stdio.h>
#include <sys/types.h>
-#include <sys/stat.h>
#include <unistd.h>
#include <fcntl.h>
#include <radeon_surface.h>

-static struct util_hash_table *fd_tab = NULL;
-static mtx_t fd_tab_mutex = _MTX_INITIALIZER_NP;

/* Enable/disable feature access for one command stream.
* If enable == true, return true on success.
@@ -558,9 +556,6 @@ static void radeon_winsys_destroy(struct radeon_winsys *rws)
mtx_destroy(&ws->bo_va_mutex);
mtx_destroy(&ws->bo_fence_lock);

- if (ws->fd >= 0)
- close(ws->fd);
-
FREE(rws);
}

@@ -680,49 +675,8 @@ static bool radeon_read_registers(struct radeon_winsys *rws,
return true;
}

-static unsigned hash_fd(void *key)
-{
- int fd = pointer_to_intptr(key);
- struct stat stat;
- fstat(fd, &stat);
-
- return stat.st_dev ^ stat.st_ino ^ stat.st_rdev;
-}
-
-static int compare_fd(void *key1, void *key2)
-{
- int fd1 = pointer_to_intptr(key1);
- int fd2 = pointer_to_intptr(key2);
- struct stat stat1, stat2;
- fstat(fd1, &stat1);
- fstat(fd2, &stat2);
-
- return stat1.st_dev != stat2.st_dev ||
- stat1.st_ino != stat2.st_ino ||
- stat1.st_rdev != stat2.st_rdev;
-}
-
DEBUG_GET_ONCE_BOOL_OPTION(thread, "RADEON_THREAD", true)

-static bool radeon_winsys_unref(struct radeon_winsys *ws)
-{
- struct radeon_drm_winsys *rws = (struct radeon_drm_winsys*)ws;
- bool destroy;
-
- /* When the reference counter drops to zero, remove the fd from the table.
- * This must happen while the mutex is locked, so that
- * radeon_drm_winsys_create in another thread doesn't get the winsys
- * from the table when the counter drops to 0. */
- mtx_lock(&fd_tab_mutex);
-
- destroy = pipe_reference(&rws->reference, NULL);
- if (destroy && fd_tab)
- util_hash_table_remove(fd_tab, intptr_to_pointer(rws->fd));
-
- mtx_unlock(&fd_tab_mutex);
- return destroy;
-}
-
#define PTR_TO_UINT(x) ((unsigned)((intptr_t)(x)))

static unsigned handle_hash(void *key)
@@ -740,24 +694,14 @@ radeon_drm_winsys_create(int fd, const struct pipe_screen_config *config,
radeon_screen_create_t screen_create)
{
struct radeon_drm_winsys *ws;
+ struct pipe_screen *pscreen = pipe_screen_reference(fd);

- mtx_lock(&fd_tab_mutex);
- if (!fd_tab) {
- fd_tab = util_hash_table_create(hash_fd, compare_fd);
- }
-
- ws = util_hash_table_get(fd_tab, intptr_to_pointer(fd));
- if (ws) {
- pipe_reference(NULL, &ws->reference);
- mtx_unlock(&fd_tab_mutex);
- return &ws->base;
- }
+ if (pscreen)
+ return pscreen->ws;

ws = CALLOC_STRUCT(radeon_drm_winsys);
- if (!ws) {
- mtx_unlock(&fd_tab_mutex);
+ if (!ws)
return NULL;
- }

ws->fd = fcntl(fd, F_DUPFD_CLOEXEC, 3);

@@ -794,11 +738,8 @@ radeon_drm_winsys_create(int fd, const struct pipe_screen_config *config,
goto fail_slab;
}

- /* init reference */
- pipe_reference_init(&ws->reference, 1);

/* Set functions. */
- ws->base.unref = radeon_winsys_unref;
ws->base.destroy = radeon_winsys_destroy;
ws->base.query_info = radeon_query_info;
ws->base.cs_request_feature = radeon_cs_request_feature;
@@ -835,16 +776,9 @@ radeon_drm_winsys_create(int fd, const struct pipe_screen_config *config,
ws->base.screen = screen_create(&ws->base, config);
if (!ws->base.screen) {
radeon_winsys_destroy(&ws->base);
- mtx_unlock(&fd_tab_mutex);
return NULL;
}
-
- util_hash_table_set(fd_tab, intptr_to_pointer(ws->fd), ws);
-
- /* We must unlock the mutex once the winsys is fully initialized, so that
- * other threads attempting to create the winsys from the same fd will
- * get a fully initialized winsys and not just half-way initialized. */
- mtx_unlock(&fd_tab_mutex);
+ ws->base.screen->ws = &ws->base;

return &ws->base;

@@ -854,7 +788,6 @@ fail_slab:
fail_cache:
pb_cache_deinit(&ws->bo_cache);
fail1:
- mtx_unlock(&fd_tab_mutex);
if (ws->surf_man)
radeon_surface_manager_free(ws->surf_man);
if (ws->fd >= 0)
diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.h b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.h
index 362dab223980..46964cae5ad2 100644
--- a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.h
+++ b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.h
@@ -50,7 +50,6 @@ enum radeon_generation {

struct radeon_drm_winsys {
struct radeon_winsys base;
- struct pipe_reference reference;
struct pb_cache bo_cache;
struct pb_slabs bo_slabs;
--
2.11.0
Marek Olšák
2017-08-09 20:19:27 UTC
Reply
Permalink
Raw Message
You already know I can't accept the amdgpu part.

Anyway, I don't know if it's possible to keep the radeon part of the
patch, because there is some shared code you would need to keep as
well.

Marek
Post by Rob Herring
Use the common pipe_screen ref counting.
radeon uses the common pipe_screen_{un}reference functions, but amdgpu
is unique in its hashing the dev pointer rather than the fd, so the
common fd hashing cannot be used. However, the same reference counting
can be used instead of the private one. The mutex can be dropped as the
pipe loader serializes the create_screen() and destroy() calls.
---
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/include/pipe/p_screen.h | 2 +
src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c | 44 ++-----------
src/gallium/winsys/amdgpu/drm/amdgpu_winsys.h | 1 -
src/gallium/winsys/radeon/drm/radeon_drm_winsys.c | 79 ++---------------------
src/gallium/winsys/radeon/drm/radeon_drm_winsys.h | 1 -
9 files changed, 13 insertions(+), 134 deletions(-)
diff --git a/src/gallium/drivers/r300/r300_screen.c b/src/gallium/drivers/r300/r300_screen.c
index ff68ffd7bc65..6991e8396638 100644
--- a/src/gallium/drivers/r300/r300_screen.c
+++ b/src/gallium/drivers/r300/r300_screen.c
@@ -696,9 +696,6 @@ static void r300_destroy_screen(struct pipe_screen* pscreen)
struct r300_screen* r300screen = r300_screen(pscreen);
struct radeon_winsys *rws = radeon_winsys(pscreen);
- if (rws && !rws->unref(rws))
- return;
-
mtx_destroy(&r300screen->cmask_mutex);
slab_destroy_parent(&r300screen->pool_transfers);
diff --git a/src/gallium/drivers/r600/r600_pipe.c b/src/gallium/drivers/r600/r600_pipe.c
index 023f1b4bd14f..c0f7312e956d 100644
--- a/src/gallium/drivers/r600/r600_pipe.c
+++ b/src/gallium/drivers/r600/r600_pipe.c
@@ -612,12 +612,6 @@ static void r600_destroy_screen(struct pipe_screen* pscreen)
{
struct r600_screen *rscreen = (struct r600_screen *)pscreen;
- if (!rscreen)
- return;
-
- if (!rscreen->b.ws->unref(rscreen->b.ws))
- return;
-
if (rscreen->global_pool) {
compute_memory_pool_delete(rscreen->global_pool);
}
diff --git a/src/gallium/drivers/radeon/radeon_winsys.h b/src/gallium/drivers/radeon/radeon_winsys.h
index 351edcd76f98..6ccd2ddbb0b2 100644
--- a/src/gallium/drivers/radeon/radeon_winsys.h
+++ b/src/gallium/drivers/radeon/radeon_winsys.h
@@ -230,14 +230,6 @@ struct radeon_winsys {
struct pipe_screen *screen;
/**
- * Decrement the winsys reference count.
- *
- * \param ws The winsys this function is called for.
- * \return True if the winsys and screen should be destroyed.
- */
- bool (*unref)(struct radeon_winsys *ws);
-
- /**
* Destroy this winsys.
*
* \param ws The winsys this function is called from.
diff --git a/src/gallium/drivers/radeonsi/si_pipe.c b/src/gallium/drivers/radeonsi/si_pipe.c
index 395853c7d9f3..881ed0c58339 100644
--- a/src/gallium/drivers/radeonsi/si_pipe.c
+++ b/src/gallium/drivers/radeonsi/si_pipe.c
@@ -843,9 +843,6 @@ static void si_destroy_screen(struct pipe_screen* pscreen)
};
unsigned i;
- if (!sscreen->b.ws->unref(sscreen->b.ws))
- return;
-
util_queue_destroy(&sscreen->shader_compiler_queue);
util_queue_destroy(&sscreen->shader_compiler_queue_low_priority);
diff --git a/src/gallium/include/pipe/p_screen.h b/src/gallium/include/pipe/p_screen.h
index dbb14ef882c0..8f866f83d763 100644
--- a/src/gallium/include/pipe/p_screen.h
+++ b/src/gallium/include/pipe/p_screen.h
@@ -71,6 +71,8 @@ struct driOptionCache;
struct pipe_screen {
struct pipe_reference reference;
int fd;
+ void *ws;
+
void (*destroy)( struct pipe_screen * );
const char *(*get_name)( struct pipe_screen * );
diff --git a/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c b/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c
index 26c7dacb9df4..258872388ce3 100644
--- a/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c
+++ b/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c
@@ -48,7 +48,6 @@
#endif
static struct util_hash_table *dev_tab = NULL;
-static mtx_t dev_tab_mutex = _MTX_INITIALIZER_NP;
/* Helper function to do the ioctls needed for setup and init. */
static bool do_winsys_init(struct amdgpu_winsys *ws, int fd)
@@ -97,6 +96,7 @@ static void amdgpu_winsys_destroy(struct radeon_winsys *rws)
pb_cache_deinit(&ws->bo_cache);
mtx_destroy(&ws->global_bo_list_lock);
do_winsys_deinit(ws);
+ util_hash_table_remove(dev_tab, ws->dev);
FREE(rws);
}
@@ -203,26 +203,6 @@ static int compare_dev(void *key1, void *key2)
return key1 != key2;
}
-static bool amdgpu_winsys_unref(struct radeon_winsys *rws)
-{
- struct amdgpu_winsys *ws = (struct amdgpu_winsys*)rws;
- bool destroy;
-
- /* When the reference counter drops to zero, remove the device pointer
- * from the table.
- * This must happen while the mutex is locked, so that
- * amdgpu_winsys_create in another thread doesn't get the winsys
- * from the table when the counter drops to 0. */
- mtx_lock(&dev_tab_mutex);
-
- destroy = pipe_reference(&ws->reference, NULL);
- if (destroy && dev_tab)
- util_hash_table_remove(dev_tab, ws->dev);
-
- mtx_unlock(&dev_tab_mutex);
- return destroy;
-}
-
static const char* amdgpu_get_chip_name(struct radeon_winsys *ws)
{
amdgpu_device_handle dev = ((struct amdgpu_winsys *)ws)->dev;
@@ -247,7 +227,6 @@ amdgpu_winsys_create(int fd, const struct pipe_screen_config *config,
drmFreeVersion(version);
/* Look up the winsys from the dev table. */
- mtx_lock(&dev_tab_mutex);
if (!dev_tab)
dev_tab = util_hash_table_create(hash_dev, compare_dev);
@@ -255,7 +234,6 @@ amdgpu_winsys_create(int fd, const struct pipe_screen_config *config,
* for the same fd. */
r = amdgpu_device_initialize(fd, &drm_major, &drm_minor, &dev);
if (r) {
- mtx_unlock(&dev_tab_mutex);
fprintf(stderr, "amdgpu: amdgpu_device_initialize failed.\n");
return NULL;
}
@@ -263,15 +241,14 @@ amdgpu_winsys_create(int fd, const struct pipe_screen_config *config,
/* Lookup a winsys if we have already created one for this device. */
ws = util_hash_table_get(dev_tab, dev);
if (ws) {
- pipe_reference(NULL, &ws->reference);
- mtx_unlock(&dev_tab_mutex);
+ pipe_reference(NULL, &ws->base.screen->reference);
return &ws->base;
}
/* Create a new winsys. */
ws = CALLOC_STRUCT(amdgpu_winsys);
if (!ws)
- goto fail;
+ return NULL;
ws->dev = dev;
ws->info.drm_major = drm_major;
@@ -296,11 +273,7 @@ amdgpu_winsys_create(int fd, const struct pipe_screen_config *config,
ws->info.min_alloc_size = 1 << AMDGPU_SLAB_MIN_SIZE_LOG2;
- /* init reference */
- pipe_reference_init(&ws->reference, 1);
-
/* Set functions. */
- ws->base.unref = amdgpu_winsys_unref;
ws->base.destroy = amdgpu_winsys_destroy;
ws->base.query_info = amdgpu_winsys_query_info;
ws->base.cs_request_feature = amdgpu_cs_request_feature;
@@ -319,7 +292,6 @@ amdgpu_winsys_create(int fd, const struct pipe_screen_config *config,
if (!util_queue_init(&ws->cs_queue, "amdgpu_cs", 8, 1,
UTIL_QUEUE_INIT_RESIZE_IF_FULL)) {
amdgpu_winsys_destroy(&ws->base);
- mtx_unlock(&dev_tab_mutex);
return NULL;
}
@@ -331,16 +303,12 @@ amdgpu_winsys_create(int fd, const struct pipe_screen_config *config,
ws->base.screen = screen_create(&ws->base, config);
if (!ws->base.screen) {
amdgpu_winsys_destroy(&ws->base);
- mtx_unlock(&dev_tab_mutex);
return NULL;
}
util_hash_table_set(dev_tab, dev, ws);
-
- /* We must unlock the mutex once the winsys is fully initialized, so that
- * other threads attempting to create the winsys from the same fd will
- * get a fully initialized winsys and not just half-way initialized. */
- mtx_unlock(&dev_tab_mutex);
+ ws->base.screen->fd = -1;
+ pipe_reference_init(&ws->base.screen->reference, 1);
return &ws->base;
do_winsys_deinit(ws);
FREE(ws);
- mtx_unlock(&dev_tab_mutex);
return NULL;
}
diff --git a/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.h b/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.h
index 7aca612f4523..52f00b33c1a6 100644
--- a/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.h
+++ b/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.h
@@ -47,7 +47,6 @@ struct amdgpu_cs;
struct amdgpu_winsys {
struct radeon_winsys base;
- struct pipe_reference reference;
struct pb_cache bo_cache;
struct pb_slabs bo_slabs;
diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
index 542e5494bebc..d27428598ed2 100644
--- a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
+++ b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
@@ -37,17 +37,15 @@
#include "util/u_memory.h"
#include "util/u_hash_table.h"
+#include "util/u_screen.h"
#include <xf86drm.h>
#include <stdio.h>
#include <sys/types.h>
-#include <sys/stat.h>
#include <unistd.h>
#include <fcntl.h>
#include <radeon_surface.h>
-static struct util_hash_table *fd_tab = NULL;
-static mtx_t fd_tab_mutex = _MTX_INITIALIZER_NP;
/* Enable/disable feature access for one command stream.
* If enable == true, return true on success.
@@ -558,9 +556,6 @@ static void radeon_winsys_destroy(struct radeon_winsys *rws)
mtx_destroy(&ws->bo_va_mutex);
mtx_destroy(&ws->bo_fence_lock);
- if (ws->fd >= 0)
- close(ws->fd);
-
FREE(rws);
}
@@ -680,49 +675,8 @@ static bool radeon_read_registers(struct radeon_winsys *rws,
return true;
}
-static unsigned hash_fd(void *key)
-{
- int fd = pointer_to_intptr(key);
- struct stat stat;
- fstat(fd, &stat);
-
- return stat.st_dev ^ stat.st_ino ^ stat.st_rdev;
-}
-
-static int compare_fd(void *key1, void *key2)
-{
- int fd1 = pointer_to_intptr(key1);
- int fd2 = pointer_to_intptr(key2);
- struct stat stat1, stat2;
- fstat(fd1, &stat1);
- fstat(fd2, &stat2);
-
- return stat1.st_dev != stat2.st_dev ||
- stat1.st_ino != stat2.st_ino ||
- stat1.st_rdev != stat2.st_rdev;
-}
-
DEBUG_GET_ONCE_BOOL_OPTION(thread, "RADEON_THREAD", true)
-static bool radeon_winsys_unref(struct radeon_winsys *ws)
-{
- struct radeon_drm_winsys *rws = (struct radeon_drm_winsys*)ws;
- bool destroy;
-
- /* When the reference counter drops to zero, remove the fd from the table.
- * This must happen while the mutex is locked, so that
- * radeon_drm_winsys_create in another thread doesn't get the winsys
- * from the table when the counter drops to 0. */
- mtx_lock(&fd_tab_mutex);
-
- destroy = pipe_reference(&rws->reference, NULL);
- if (destroy && fd_tab)
- util_hash_table_remove(fd_tab, intptr_to_pointer(rws->fd));
-
- mtx_unlock(&fd_tab_mutex);
- return destroy;
-}
-
#define PTR_TO_UINT(x) ((unsigned)((intptr_t)(x)))
static unsigned handle_hash(void *key)
@@ -740,24 +694,14 @@ radeon_drm_winsys_create(int fd, const struct pipe_screen_config *config,
radeon_screen_create_t screen_create)
{
struct radeon_drm_winsys *ws;
+ struct pipe_screen *pscreen = pipe_screen_reference(fd);
- mtx_lock(&fd_tab_mutex);
- if (!fd_tab) {
- fd_tab = util_hash_table_create(hash_fd, compare_fd);
- }
-
- ws = util_hash_table_get(fd_tab, intptr_to_pointer(fd));
- if (ws) {
- pipe_reference(NULL, &ws->reference);
- mtx_unlock(&fd_tab_mutex);
- return &ws->base;
- }
+ if (pscreen)
+ return pscreen->ws;
ws = CALLOC_STRUCT(radeon_drm_winsys);
- if (!ws) {
- mtx_unlock(&fd_tab_mutex);
+ if (!ws)
return NULL;
- }
ws->fd = fcntl(fd, F_DUPFD_CLOEXEC, 3);
@@ -794,11 +738,8 @@ radeon_drm_winsys_create(int fd, const struct pipe_screen_config *config,
goto fail_slab;
}
- /* init reference */
- pipe_reference_init(&ws->reference, 1);
/* Set functions. */
- ws->base.unref = radeon_winsys_unref;
ws->base.destroy = radeon_winsys_destroy;
ws->base.query_info = radeon_query_info;
ws->base.cs_request_feature = radeon_cs_request_feature;
@@ -835,16 +776,9 @@ radeon_drm_winsys_create(int fd, const struct pipe_screen_config *config,
ws->base.screen = screen_create(&ws->base, config);
if (!ws->base.screen) {
radeon_winsys_destroy(&ws->base);
- mtx_unlock(&fd_tab_mutex);
return NULL;
}
-
- util_hash_table_set(fd_tab, intptr_to_pointer(ws->fd), ws);
-
- /* We must unlock the mutex once the winsys is fully initialized, so that
- * other threads attempting to create the winsys from the same fd will
- * get a fully initialized winsys and not just half-way initialized. */
- mtx_unlock(&fd_tab_mutex);
+ ws->base.screen->ws = &ws->base;
return &ws->base;
pb_cache_deinit(&ws->bo_cache);
- mtx_unlock(&fd_tab_mutex);
if (ws->surf_man)
radeon_surface_manager_free(ws->surf_man);
if (ws->fd >= 0)
diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.h b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.h
index 362dab223980..46964cae5ad2 100644
--- a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.h
+++ b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.h
@@ -50,7 +50,6 @@ enum radeon_generation {
struct radeon_drm_winsys {
struct radeon_winsys base;
- struct pipe_reference reference;
struct pb_cache bo_cache;
struct pb_slabs bo_slabs;
--
2.11.0
_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Rob Herring
2017-08-09 20:34:53 UTC
Reply
Permalink
Raw Message
Post by Marek Olšák
You already know I can't accept the amdgpu part.
You don't agree with my explanation in the other patch?
Post by Marek Olšák
Anyway, I don't know if it's possible to keep the radeon part of the
patch, because there is some shared code you would need to keep as
well.
Yes, I think it should be possible.

Rob
Marek Olšák
2017-08-11 18:14:14 UTC
Reply
Permalink
Raw Message
Post by Rob Herring
Post by Marek Olšák
You already know I can't accept the amdgpu part.
You don't agree with my explanation in the other patch?
OK. This patch is:

Reviewed-by: Marek Olšák <***@amd.com>

Marek

Rob Herring
2017-08-07 22:58:12 UTC
Reply
Permalink
Raw Message
Use the common pipe_screen ref counting.

radeon uses the common pipe_screen_{un}reference functions, but amdgpu
is unique in its hashing the dev pointer rather than the fd, so the
common fd hashing cannot be used. However, the same reference counting
can be used instead of the private one. The mutex can be dropped as the
pipe loader serializes the create_screen() and destroy() calls.

Signed-off-by: Rob Herring <***@kernel.org>
Cc: "Marek Olšák" <***@amd.com>
Cc: Ilia Mirkin <***@alum.mit.edu>
---
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/include/pipe/p_screen.h | 2 +
src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c | 44 ++-----------
src/gallium/winsys/amdgpu/drm/amdgpu_winsys.h | 1 -
src/gallium/winsys/radeon/drm/radeon_drm_winsys.c | 79 ++---------------------
src/gallium/winsys/radeon/drm/radeon_drm_winsys.h | 1 -
9 files changed, 13 insertions(+), 134 deletions(-)

diff --git a/src/gallium/drivers/r300/r300_screen.c b/src/gallium/drivers/r300/r300_screen.c
index ff68ffd7bc65..6991e8396638 100644
--- a/src/gallium/drivers/r300/r300_screen.c
+++ b/src/gallium/drivers/r300/r300_screen.c
@@ -696,9 +696,6 @@ static void r300_destroy_screen(struct pipe_screen* pscreen)
struct r300_screen* r300screen = r300_screen(pscreen);
struct radeon_winsys *rws = radeon_winsys(pscreen);

- if (rws && !rws->unref(rws))
- return;
-
mtx_destroy(&r300screen->cmask_mutex);
slab_destroy_parent(&r300screen->pool_transfers);

diff --git a/src/gallium/drivers/r600/r600_pipe.c b/src/gallium/drivers/r600/r600_pipe.c
index 023f1b4bd14f..c0f7312e956d 100644
--- a/src/gallium/drivers/r600/r600_pipe.c
+++ b/src/gallium/drivers/r600/r600_pipe.c
@@ -612,12 +612,6 @@ static void r600_destroy_screen(struct pipe_screen* pscreen)
{
struct r600_screen *rscreen = (struct r600_screen *)pscreen;

- if (!rscreen)
- return;
-
- if (!rscreen->b.ws->unref(rscreen->b.ws))
- return;
-
if (rscreen->global_pool) {
compute_memory_pool_delete(rscreen->global_pool);
}
diff --git a/src/gallium/drivers/radeon/radeon_winsys.h b/src/gallium/drivers/radeon/radeon_winsys.h
index 351edcd76f98..6ccd2ddbb0b2 100644
--- a/src/gallium/drivers/radeon/radeon_winsys.h
+++ b/src/gallium/drivers/radeon/radeon_winsys.h
@@ -230,14 +230,6 @@ struct radeon_winsys {
struct pipe_screen *screen;

/**
- * Decrement the winsys reference count.
- *
- * \param ws The winsys this function is called for.
- * \return True if the winsys and screen should be destroyed.
- */
- bool (*unref)(struct radeon_winsys *ws);
-
- /**
* Destroy this winsys.
*
* \param ws The winsys this function is called from.
diff --git a/src/gallium/drivers/radeonsi/si_pipe.c b/src/gallium/drivers/radeonsi/si_pipe.c
index 395853c7d9f3..881ed0c58339 100644
--- a/src/gallium/drivers/radeonsi/si_pipe.c
+++ b/src/gallium/drivers/radeonsi/si_pipe.c
@@ -843,9 +843,6 @@ static void si_destroy_screen(struct pipe_screen* pscreen)
};
unsigned i;

- if (!sscreen->b.ws->unref(sscreen->b.ws))
- return;
-
util_queue_destroy(&sscreen->shader_compiler_queue);
util_queue_destroy(&sscreen->shader_compiler_queue_low_priority);

diff --git a/src/gallium/include/pipe/p_screen.h b/src/gallium/include/pipe/p_screen.h
index dbb14ef882c0..8f866f83d763 100644
--- a/src/gallium/include/pipe/p_screen.h
+++ b/src/gallium/include/pipe/p_screen.h
@@ -71,6 +71,8 @@ struct driOptionCache;
struct pipe_screen {
struct pipe_reference reference;
int fd;
+ void *ws;
+
void (*destroy)( struct pipe_screen * );

const char *(*get_name)( struct pipe_screen * );
diff --git a/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c b/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c
index 26c7dacb9df4..258872388ce3 100644
--- a/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c
+++ b/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c
@@ -48,7 +48,6 @@
#endif

static struct util_hash_table *dev_tab = NULL;
-static mtx_t dev_tab_mutex = _MTX_INITIALIZER_NP;

/* Helper function to do the ioctls needed for setup and init. */
static bool do_winsys_init(struct amdgpu_winsys *ws, int fd)
@@ -97,6 +96,7 @@ static void amdgpu_winsys_destroy(struct radeon_winsys *rws)
pb_cache_deinit(&ws->bo_cache);
mtx_destroy(&ws->global_bo_list_lock);
do_winsys_deinit(ws);
+ util_hash_table_remove(dev_tab, ws->dev);
FREE(rws);
}

@@ -203,26 +203,6 @@ static int compare_dev(void *key1, void *key2)
return key1 != key2;
}

-static bool amdgpu_winsys_unref(struct radeon_winsys *rws)
-{
- struct amdgpu_winsys *ws = (struct amdgpu_winsys*)rws;
- bool destroy;
-
- /* When the reference counter drops to zero, remove the device pointer
- * from the table.
- * This must happen while the mutex is locked, so that
- * amdgpu_winsys_create in another thread doesn't get the winsys
- * from the table when the counter drops to 0. */
- mtx_lock(&dev_tab_mutex);
-
- destroy = pipe_reference(&ws->reference, NULL);
- if (destroy && dev_tab)
- util_hash_table_remove(dev_tab, ws->dev);
-
- mtx_unlock(&dev_tab_mutex);
- return destroy;
-}
-
static const char* amdgpu_get_chip_name(struct radeon_winsys *ws)
{
amdgpu_device_handle dev = ((struct amdgpu_winsys *)ws)->dev;
@@ -247,7 +227,6 @@ amdgpu_winsys_create(int fd, const struct pipe_screen_config *config,
drmFreeVersion(version);

/* Look up the winsys from the dev table. */
- mtx_lock(&dev_tab_mutex);
if (!dev_tab)
dev_tab = util_hash_table_create(hash_dev, compare_dev);

@@ -255,7 +234,6 @@ amdgpu_winsys_create(int fd, const struct pipe_screen_config *config,
* for the same fd. */
r = amdgpu_device_initialize(fd, &drm_major, &drm_minor, &dev);
if (r) {
- mtx_unlock(&dev_tab_mutex);
fprintf(stderr, "amdgpu: amdgpu_device_initialize failed.\n");
return NULL;
}
@@ -263,15 +241,14 @@ amdgpu_winsys_create(int fd, const struct pipe_screen_config *config,
/* Lookup a winsys if we have already created one for this device. */
ws = util_hash_table_get(dev_tab, dev);
if (ws) {
- pipe_reference(NULL, &ws->reference);
- mtx_unlock(&dev_tab_mutex);
+ pipe_reference(NULL, &ws->base.screen->reference);
return &ws->base;
}

/* Create a new winsys. */
ws = CALLOC_STRUCT(amdgpu_winsys);
if (!ws)
- goto fail;
+ return NULL;

ws->dev = dev;
ws->info.drm_major = drm_major;
@@ -296,11 +273,7 @@ amdgpu_winsys_create(int fd, const struct pipe_screen_config *config,

ws->info.min_alloc_size = 1 << AMDGPU_SLAB_MIN_SIZE_LOG2;

- /* init reference */
- pipe_reference_init(&ws->reference, 1);
-
/* Set functions. */
- ws->base.unref = amdgpu_winsys_unref;
ws->base.destroy = amdgpu_winsys_destroy;
ws->base.query_info = amdgpu_winsys_query_info;
ws->base.cs_request_feature = amdgpu_cs_request_feature;
@@ -319,7 +292,6 @@ amdgpu_winsys_create(int fd, const struct pipe_screen_config *config,
if (!util_queue_init(&ws->cs_queue, "amdgpu_cs", 8, 1,
UTIL_QUEUE_INIT_RESIZE_IF_FULL)) {
amdgpu_winsys_destroy(&ws->base);
- mtx_unlock(&dev_tab_mutex);
return NULL;
}

@@ -331,16 +303,12 @@ amdgpu_winsys_create(int fd, const struct pipe_screen_config *config,
ws->base.screen = screen_create(&ws->base, config);
if (!ws->base.screen) {
amdgpu_winsys_destroy(&ws->base);
- mtx_unlock(&dev_tab_mutex);
return NULL;
}

util_hash_table_set(dev_tab, dev, ws);
-
- /* We must unlock the mutex once the winsys is fully initialized, so that
- * other threads attempting to create the winsys from the same fd will
- * get a fully initialized winsys and not just half-way initialized. */
- mtx_unlock(&dev_tab_mutex);
+ ws->base.screen->fd = -1;
+ pipe_reference_init(&ws->base.screen->reference, 1);

return &ws->base;

@@ -349,7 +317,5 @@ fail_cache:
do_winsys_deinit(ws);
fail_alloc:
FREE(ws);
-fail:
- mtx_unlock(&dev_tab_mutex);
return NULL;
}
diff --git a/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.h b/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.h
index 7aca612f4523..52f00b33c1a6 100644
--- a/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.h
+++ b/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.h
@@ -47,7 +47,6 @@ struct amdgpu_cs;

struct amdgpu_winsys {
struct radeon_winsys base;
- struct pipe_reference reference;
struct pb_cache bo_cache;
struct pb_slabs bo_slabs;

diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
index 542e5494bebc..d27428598ed2 100644
--- a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
+++ b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
@@ -37,17 +37,15 @@

#include "util/u_memory.h"
#include "util/u_hash_table.h"
+#include "util/u_screen.h"

#include <xf86drm.h>
#include <stdio.h>
#include <sys/types.h>
-#include <sys/stat.h>
#include <unistd.h>
#include <fcntl.h>
#include <radeon_surface.h>

-static struct util_hash_table *fd_tab = NULL;
-static mtx_t fd_tab_mutex = _MTX_INITIALIZER_NP;

/* Enable/disable feature access for one command stream.
* If enable == true, return true on success.
@@ -558,9 +556,6 @@ static void radeon_winsys_destroy(struct radeon_winsys *rws)
mtx_destroy(&ws->bo_va_mutex);
mtx_destroy(&ws->bo_fence_lock);

- if (ws->fd >= 0)
- close(ws->fd);
-
FREE(rws);
}

@@ -680,49 +675,8 @@ static bool radeon_read_registers(struct radeon_winsys *rws,
return true;
}

-static unsigned hash_fd(void *key)
-{
- int fd = pointer_to_intptr(key);
- struct stat stat;
- fstat(fd, &stat);
-
- return stat.st_dev ^ stat.st_ino ^ stat.st_rdev;
-}
-
-static int compare_fd(void *key1, void *key2)
-{
- int fd1 = pointer_to_intptr(key1);
- int fd2 = pointer_to_intptr(key2);
- struct stat stat1, stat2;
- fstat(fd1, &stat1);
- fstat(fd2, &stat2);
-
- return stat1.st_dev != stat2.st_dev ||
- stat1.st_ino != stat2.st_ino ||
- stat1.st_rdev != stat2.st_rdev;
-}
-
DEBUG_GET_ONCE_BOOL_OPTION(thread, "RADEON_THREAD", true)

-static bool radeon_winsys_unref(struct radeon_winsys *ws)
-{
- struct radeon_drm_winsys *rws = (struct radeon_drm_winsys*)ws;
- bool destroy;
-
- /* When the reference counter drops to zero, remove the fd from the table.
- * This must happen while the mutex is locked, so that
- * radeon_drm_winsys_create in another thread doesn't get the winsys
- * from the table when the counter drops to 0. */
- mtx_lock(&fd_tab_mutex);
-
- destroy = pipe_reference(&rws->reference, NULL);
- if (destroy && fd_tab)
- util_hash_table_remove(fd_tab, intptr_to_pointer(rws->fd));
-
- mtx_unlock(&fd_tab_mutex);
- return destroy;
-}
-
#define PTR_TO_UINT(x) ((unsigned)((intptr_t)(x)))

static unsigned handle_hash(void *key)
@@ -740,24 +694,14 @@ radeon_drm_winsys_create(int fd, const struct pipe_screen_config *config,
radeon_screen_create_t screen_create)
{
struct radeon_drm_winsys *ws;
+ struct pipe_screen *pscreen = pipe_screen_reference(fd);

- mtx_lock(&fd_tab_mutex);
- if (!fd_tab) {
- fd_tab = util_hash_table_create(hash_fd, compare_fd);
- }
-
- ws = util_hash_table_get(fd_tab, intptr_to_pointer(fd));
- if (ws) {
- pipe_reference(NULL, &ws->reference);
- mtx_unlock(&fd_tab_mutex);
- return &ws->base;
- }
+ if (pscreen)
+ return pscreen->ws;

ws = CALLOC_STRUCT(radeon_drm_winsys);
- if (!ws) {
- mtx_unlock(&fd_tab_mutex);
+ if (!ws)
return NULL;
- }

ws->fd = fcntl(fd, F_DUPFD_CLOEXEC, 3);

@@ -794,11 +738,8 @@ radeon_drm_winsys_create(int fd, const struct pipe_screen_config *config,
goto fail_slab;
}

- /* init reference */
- pipe_reference_init(&ws->reference, 1);

/* Set functions. */
- ws->base.unref = radeon_winsys_unref;
ws->base.destroy = radeon_winsys_destroy;
ws->base.query_info = radeon_query_info;
ws->base.cs_request_feature = radeon_cs_request_feature;
@@ -835,16 +776,9 @@ radeon_drm_winsys_create(int fd, const struct pipe_screen_config *config,
ws->base.screen = screen_create(&ws->base, config);
if (!ws->base.screen) {
radeon_winsys_destroy(&ws->base);
- mtx_unlock(&fd_tab_mutex);
return NULL;
}
-
- util_hash_table_set(fd_tab, intptr_to_pointer(ws->fd), ws);
-
- /* We must unlock the mutex once the winsys is fully initialized, so that
- * other threads attempting to create the winsys from the same fd will
- * get a fully initialized winsys and not just half-way initialized. */
- mtx_unlock(&fd_tab_mutex);
+ ws->base.screen->ws = &ws->base;

return &ws->base;

@@ -854,7 +788,6 @@ fail_slab:
fail_cache:
pb_cache_deinit(&ws->bo_cache);
fail1:
- mtx_unlock(&fd_tab_mutex);
if (ws->surf_man)
radeon_surface_manager_free(ws->surf_man);
if (ws->fd >= 0)
diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.h b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.h
index 362dab223980..46964cae5ad2 100644
--- a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.h
+++ b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.h
@@ -50,7 +50,6 @@ enum radeon_generation {

struct radeon_drm_winsys {
struct radeon_winsys base;
- struct pipe_reference reference;
struct pb_cache bo_cache;
struct pb_slabs bo_slabs;
--
2.11.0
Rob Herring
2017-08-07 22:58:14 UTC
Reply
Permalink
Raw Message
Use the common pipe_screen ref counting and fd hashing functions.
The mutex can be dropped as the pipe loader serializes the
create_screen() and destroy() calls.

Signed-off-by: Rob Herring <***@kernel.org>
---
src/gallium/auxiliary/target-helpers/drm_helper.h | 2 +-
src/gallium/drivers/svga/svga_public.h | 2 +-
src/gallium/drivers/svga/svga_screen.c | 7 ++-
src/gallium/targets/pipe-loader/pipe_vmwgfx.c | 2 +-
src/gallium/winsys/svga/drm/vmw_screen.c | 57 +++++------------------
src/gallium/winsys/svga/drm/vmw_screen.h | 6 ---
6 files changed, 20 insertions(+), 56 deletions(-)

diff --git a/src/gallium/auxiliary/target-helpers/drm_helper.h b/src/gallium/auxiliary/target-helpers/drm_helper.h
index 95b4a27111cb..8831ab3cb658 100644
--- a/src/gallium/auxiliary/target-helpers/drm_helper.h
+++ b/src/gallium/auxiliary/target-helpers/drm_helper.h
@@ -225,7 +225,7 @@ pipe_vmwgfx_create_screen(int fd, const struct pipe_screen_config *config)
if (!sws)
return NULL;

- screen = svga_screen_create(sws);
+ screen = svga_screen_create(sws, fd);
return screen ? debug_screen_wrap(screen) : NULL;
}

diff --git a/src/gallium/drivers/svga/svga_public.h b/src/gallium/drivers/svga/svga_public.h
index ded2e2482a7f..5a95660586d2 100644
--- a/src/gallium/drivers/svga/svga_public.h
+++ b/src/gallium/drivers/svga/svga_public.h
@@ -37,6 +37,6 @@ struct pipe_screen;
struct svga_winsys_screen;

struct pipe_screen *
-svga_screen_create(struct svga_winsys_screen *sws);
+svga_screen_create(struct svga_winsys_screen *sws, int fd);

#endif /* SVGA_PUBLIC_H_ */
diff --git a/src/gallium/drivers/svga/svga_screen.c b/src/gallium/drivers/svga/svga_screen.c
index ef3e07ea38f4..7edc36f87fae 100644
--- a/src/gallium/drivers/svga/svga_screen.c
+++ b/src/gallium/drivers/svga/svga_screen.c
@@ -29,6 +29,7 @@
#include "util/u_inlines.h"
#include "util/u_string.h"
#include "util/u_math.h"
+#include "util/u_screen.h"

#include "os/os_process.h"

@@ -46,6 +47,8 @@
#include "svga3d_shaderdefs.h"
#include "VGPU10ShaderTokens.h"

+#include <fcntl.h>
+
/* NOTE: this constant may get moved into a svga3d*.h header file */
#define SVGA3D_DX_MAX_RESOURCE_SIZE (128 * 1024 * 1024)

@@ -999,7 +1002,7 @@ svga_destroy_screen( struct pipe_screen *screen )
* Create a new svga_screen object
*/
struct pipe_screen *
-svga_screen_create(struct svga_winsys_screen *sws)
+svga_screen_create(struct svga_winsys_screen *sws, int fd)
{
struct svga_screen *svgascreen;
struct pipe_screen *screen;
@@ -1192,6 +1195,8 @@ svga_screen_create(struct svga_winsys_screen *sws)

init_logging(screen);

+ pipe_screen_reference_init(screen, fcntl(fd, F_DUPFD_CLOEXEC, 3));
+
return screen;
error2:
FREE(svgascreen);
diff --git a/src/gallium/targets/pipe-loader/pipe_vmwgfx.c b/src/gallium/targets/pipe-loader/pipe_vmwgfx.c
index 68bf92ce82e5..979401161176 100644
--- a/src/gallium/targets/pipe-loader/pipe_vmwgfx.c
+++ b/src/gallium/targets/pipe-loader/pipe_vmwgfx.c
@@ -14,7 +14,7 @@ create_screen(int fd, const struct pipe_screen_config *config)
if (!sws)
return NULL;

- screen = svga_screen_create(sws);
+ screen = svga_screen_create(sws, fd);
if (!screen)
return NULL;

diff --git a/src/gallium/winsys/svga/drm/vmw_screen.c b/src/gallium/winsys/svga/drm/vmw_screen.c
index e122e0c9902c..94a058abc162 100644
--- a/src/gallium/winsys/svga/drm/vmw_screen.c
+++ b/src/gallium/winsys/svga/drm/vmw_screen.c
@@ -29,31 +29,17 @@
#include "vmw_context.h"

#include "util/u_memory.h"
+#include "util/u_screen.h"
#include "pipe/p_compiler.h"
-#include "util/u_hash_table.h"
#ifdef MAJOR_IN_MKDEV
#include <sys/mkdev.h>
#endif
#ifdef MAJOR_IN_SYSMACROS
#include <sys/sysmacros.h>
#endif
-#include <sys/stat.h>
#include <unistd.h>
#include <fcntl.h>

-static struct util_hash_table *dev_hash = NULL;
-
-static int vmw_dev_compare(void *key1, void *key2)
-{
- return (major(*(dev_t *)key1) == major(*(dev_t *)key2) &&
- minor(*(dev_t *)key1) == minor(*(dev_t *)key2)) ? 0 : 1;
-}
-
-static unsigned vmw_dev_hash(void *key)
-{
- return (major(*(dev_t *) key) << 16) | minor(*(dev_t *) key);
-}
-
/* Called from vmw_drm_create_screen(), creates and initializes the
* vmw_winsys_screen structure, which is the main entity in this
* module.
@@ -66,29 +52,15 @@ struct vmw_winsys_screen *
vmw_winsys_create( int fd )
{
struct vmw_winsys_screen *vws;
- struct stat stat_buf;
-
- if (dev_hash == NULL) {
- dev_hash = util_hash_table_create(vmw_dev_hash, vmw_dev_compare);
- if (dev_hash == NULL)
- return NULL;
- }
+ struct pipe_screen *pscreen = pipe_screen_reference(fd);

- if (fstat(fd, &stat_buf))
- return NULL;
-
- vws = util_hash_table_get(dev_hash, &stat_buf.st_rdev);
- if (vws) {
- vws->open_count++;
- return vws;
- }
+ if (pscreen)
+ return vmw_winsys_screen(svga_winsys_screen(pscreen));

vws = CALLOC_STRUCT(vmw_winsys_screen);
if (!vws)
goto out_no_vws;

- vws->device = stat_buf.st_rdev;
- vws->open_count = 1;
vws->ioctl.drm_fd = fcntl(fd, F_DUPFD_CLOEXEC, 3);
vws->base.have_gb_dma = TRUE;
vws->base.need_to_rebind_resources = FALSE;
@@ -106,14 +78,10 @@ vmw_winsys_create( int fd )
if (!vmw_winsys_screen_init_svga(vws))
goto out_no_svga;

- if (util_hash_table_set(dev_hash, &vws->device, vws) != PIPE_OK)
- goto out_no_hash_insert;
-
cnd_init(&vws->cs_cond);
mtx_init(&vws->cs_mutex, mtx_plain);

return vws;
-out_no_hash_insert:
out_no_svga:
vmw_pools_cleanup(vws);
out_no_pools:
@@ -130,14 +98,11 @@ out_no_vws:
void
vmw_winsys_destroy(struct vmw_winsys_screen *vws)
{
- if (--vws->open_count == 0) {
- util_hash_table_remove(dev_hash, &vws->device);
- vmw_pools_cleanup(vws);
- vws->fence_ops->destroy(vws->fence_ops);
- vmw_ioctl_cleanup(vws);
- close(vws->ioctl.drm_fd);
- mtx_destroy(&vws->cs_mutex);
- cnd_destroy(&vws->cs_cond);
- FREE(vws);
- }
+ vmw_pools_cleanup(vws);
+ vws->fence_ops->destroy(vws->fence_ops);
+ vmw_ioctl_cleanup(vws);
+ close(vws->ioctl.drm_fd);
+ mtx_destroy(&vws->cs_mutex);
+ cnd_destroy(&vws->cs_cond);
+ FREE(vws);
}
diff --git a/src/gallium/winsys/svga/drm/vmw_screen.h b/src/gallium/winsys/svga/drm/vmw_screen.h
index f21cabb51f93..e5faf36e73dc 100644
--- a/src/gallium/winsys/svga/drm/vmw_screen.h
+++ b/src/gallium/winsys/svga/drm/vmw_screen.h
@@ -94,12 +94,6 @@ struct vmw_winsys_screen

struct pb_fence_ops *fence_ops;

- /*
- * Screen instances
- */
- dev_t device;
- int open_count;
-
cnd_t cs_cond;
mtx_t cs_mutex;
};
--
2.11.0
Rob Herring
2017-08-07 22:58:15 UTC
Reply
Permalink
Raw Message
Use the common pipe_screen ref counting and fd hashing functions.
The mutex can be dropped as the pipe loader serializes the
create_screen() and destroy() calls.

Signed-off-by: Rob Herring <***@kernel.org>
Cc: Dave Airlie <***@redhat.com>
---
src/gallium/winsys/virgl/drm/virgl_drm_winsys.c | 88 +++----------------------
1 file changed, 10 insertions(+), 78 deletions(-)

diff --git a/src/gallium/winsys/virgl/drm/virgl_drm_winsys.c b/src/gallium/winsys/virgl/drm/virgl_drm_winsys.c
index 7f542e7f1ff4..c6795afb0698 100644
--- a/src/gallium/winsys/virgl/drm/virgl_drm_winsys.c
+++ b/src/gallium/winsys/virgl/drm/virgl_drm_winsys.c
@@ -25,7 +25,6 @@
#include <fcntl.h>
#include <stdio.h>
#include <sys/ioctl.h>
-#include <sys/stat.h>

#include "os/os_mman.h"
#include "os/os_time.h"
@@ -33,6 +32,7 @@
#include "util/u_format.h"
#include "util/u_hash_table.h"
#include "util/u_inlines.h"
+#include "util/u_screen.h"
#include "state_tracker/drm_driver.h"
#include "virgl/virgl_screen.h"
#include "virgl/virgl_public.h"
@@ -815,86 +815,18 @@ virgl_drm_winsys_create(int drmFD)

}

-static struct util_hash_table *fd_tab = NULL;
-static mtx_t virgl_screen_mutex = _MTX_INITIALIZER_NP;
-
-static void
-virgl_drm_screen_destroy(struct pipe_screen *pscreen)
-{
- struct virgl_screen *screen = virgl_screen(pscreen);
- boolean destroy;
-
- mtx_lock(&virgl_screen_mutex);
- destroy = --screen->refcnt == 0;
- if (destroy) {
- int fd = virgl_drm_winsys(screen->vws)->fd;
- util_hash_table_remove(fd_tab, intptr_to_pointer(fd));
- }
- mtx_unlock(&virgl_screen_mutex);
-
- if (destroy) {
- pscreen->destroy = screen->winsys_priv;
- pscreen->destroy(pscreen);
- }
-}
-
-static unsigned hash_fd(void *key)
-{
- int fd = pointer_to_intptr(key);
- struct stat stat;
- fstat(fd, &stat);
-
- return stat.st_dev ^ stat.st_ino ^ stat.st_rdev;
-}
-
-static int compare_fd(void *key1, void *key2)
-{
- int fd1 = pointer_to_intptr(key1);
- int fd2 = pointer_to_intptr(key2);
- struct stat stat1, stat2;
- fstat(fd1, &stat1);
- fstat(fd2, &stat2);
-
- return stat1.st_dev != stat2.st_dev ||
- stat1.st_ino != stat2.st_ino ||
- stat1.st_rdev != stat2.st_rdev;
-}
-
struct pipe_screen *
virgl_drm_screen_create(int fd)
{
- struct pipe_screen *pscreen = NULL;
-
- mtx_lock(&virgl_screen_mutex);
- if (!fd_tab) {
- fd_tab = util_hash_table_create(hash_fd, compare_fd);
- if (!fd_tab)
- goto unlock;
- }
-
- pscreen = util_hash_table_get(fd_tab, intptr_to_pointer(fd));
- if (pscreen) {
- virgl_screen(pscreen)->refcnt++;
- } else {
- struct virgl_winsys *vws;
- int dup_fd = fcntl(fd, F_DUPFD_CLOEXEC, 3);
-
- vws = virgl_drm_winsys_create(dup_fd);
-
- pscreen = virgl_create_screen(vws);
- if (pscreen) {
- util_hash_table_set(fd_tab, intptr_to_pointer(dup_fd), pscreen);
-
- /* Bit of a hack, to avoid circular linkage dependency,
- * ie. pipe driver having to call in to winsys, we
- * override the pipe drivers screen->destroy():
- */
- virgl_screen(pscreen)->winsys_priv = pscreen->destroy;
- pscreen->destroy = virgl_drm_screen_destroy;
- }
- }
+ int dupfd;
+ struct virgl_winsys *vws;
+ struct pipe_screen *pscreen = pipe_screen_reference(fd);
+ if (pscreen)
+ return pscreen;

-unlock:
- mtx_unlock(&virgl_screen_mutex);
+ dupfd = fcntl(fd, F_DUPFD_CLOEXEC, 3);
+ vws = virgl_drm_winsys_create(dupfd);
+ pscreen = virgl_create_screen(vws);
+ pipe_screen_reference_init(pscreen, dupfd);
return pscreen;
}
--
2.11.0
Rob Herring
2017-08-07 22:58:16 UTC
Reply
Permalink
Raw Message
Use the common pipe_screen ref counting and fd hashing functions.
The mutex can be dropped as the pipe loader serializes the
create_screen() and destroy() calls.

This changes the reference counting from the winsys to the pipe screen.
Previously, it was possible to have 1 winsys with multiple pipe screens.
Now there will only be a single winsys and pipe screen.

Signed-off-by: Rob Herring <***@kernel.org>
Cc: Brian Paul <***@vmware.com>
---
src/gallium/auxiliary/target-helpers/drm_helper.h | 21 ++++++---
src/gallium/targets/pipe-loader/pipe_vmwgfx.c | 24 +++++-----
src/gallium/winsys/svga/drm/vmw_screen.c | 55 ++++-------------------
src/gallium/winsys/svga/drm/vmw_screen.h | 6 ---
4 files changed, 37 insertions(+), 69 deletions(-)

diff --git a/src/gallium/auxiliary/target-helpers/drm_helper.h b/src/gallium/auxiliary/target-helpers/drm_helper.h
index 95b4a27111cb..dd5ca613ba1f 100644
--- a/src/gallium/auxiliary/target-helpers/drm_helper.h
+++ b/src/gallium/auxiliary/target-helpers/drm_helper.h
@@ -214,19 +214,28 @@ pipe_radeonsi_configuration_query(enum drm_conf conf)
#ifdef GALLIUM_VMWGFX
#include "svga/drm/svga_drm_public.h"
#include "svga/svga_public.h"
+#include "util/u_screen.h"
+
+#include <fcntl.h>

struct pipe_screen *
pipe_vmwgfx_create_screen(int fd, const struct pipe_screen_config *config)
{
struct svga_winsys_screen *sws;
- struct pipe_screen *screen;
+ struct pipe_screen *screen = pipe_screen_reference(fd);

- sws = svga_drm_winsys_screen_create(fd);
- if (!sws)
- return NULL;
+ if (!screen) {
+ sws = svga_drm_winsys_screen_create(fd);
+ if (!sws)
+ return NULL;

- screen = svga_screen_create(sws);
- return screen ? debug_screen_wrap(screen) : NULL;
+ screen = svga_screen_create(sws);
+ if (!screen)
+ return NULL;
+
+ pipe_screen_reference_init(screen, fcntl(fd, F_DUPFD_CLOEXEC, 3));
+ }
+ return debug_screen_wrap(screen);
}

#else
diff --git a/src/gallium/targets/pipe-loader/pipe_vmwgfx.c b/src/gallium/targets/pipe-loader/pipe_vmwgfx.c
index 68bf92ce82e5..0e7baf3f1552 100644
--- a/src/gallium/targets/pipe-loader/pipe_vmwgfx.c
+++ b/src/gallium/targets/pipe-loader/pipe_vmwgfx.c
@@ -3,24 +3,28 @@
#include "state_tracker/drm_driver.h"
#include "svga/drm/svga_drm_public.h"
#include "svga/svga_public.h"
+#include "util/u_screen.h"
+
+#include <fcntl.h>

static struct pipe_screen *
create_screen(int fd, const struct pipe_screen_config *config)
{
struct svga_winsys_screen *sws;
- struct pipe_screen *screen;
-
- sws = svga_drm_winsys_screen_create(fd);
- if (!sws)
- return NULL;
+ struct pipe_screen *screen = pipe_screen_reference(fd);

- screen = svga_screen_create(sws);
- if (!screen)
- return NULL;
+ if (!screen) {
+ sws = svga_drm_winsys_screen_create(fd);
+ if (!sws)
+ return NULL;

- screen = debug_screen_wrap(screen);
+ screen = svga_screen_create(sws);
+ if (!screen)
+ return NULL;

- return screen;
+ pipe_screen_reference_init(screen, fcntl(fd, F_DUPFD_CLOEXEC, 3));
+ }
+ return debug_screen_wrap(screen);
}

static const struct drm_conf_ret throttle_ret = {
diff --git a/src/gallium/winsys/svga/drm/vmw_screen.c b/src/gallium/winsys/svga/drm/vmw_screen.c
index e122e0c9902c..90fb8149c204 100644
--- a/src/gallium/winsys/svga/drm/vmw_screen.c
+++ b/src/gallium/winsys/svga/drm/vmw_screen.c
@@ -29,31 +29,17 @@
#include "vmw_context.h"

#include "util/u_memory.h"
+#include "util/u_screen.h"
#include "pipe/p_compiler.h"
-#include "util/u_hash_table.h"
#ifdef MAJOR_IN_MKDEV
#include <sys/mkdev.h>
#endif
#ifdef MAJOR_IN_SYSMACROS
#include <sys/sysmacros.h>
#endif
-#include <sys/stat.h>
#include <unistd.h>
#include <fcntl.h>

-static struct util_hash_table *dev_hash = NULL;
-
-static int vmw_dev_compare(void *key1, void *key2)
-{
- return (major(*(dev_t *)key1) == major(*(dev_t *)key2) &&
- minor(*(dev_t *)key1) == minor(*(dev_t *)key2)) ? 0 : 1;
-}
-
-static unsigned vmw_dev_hash(void *key)
-{
- return (major(*(dev_t *) key) << 16) | minor(*(dev_t *) key);
-}
-
/* Called from vmw_drm_create_screen(), creates and initializes the
* vmw_winsys_screen structure, which is the main entity in this
* module.
@@ -66,29 +52,11 @@ struct vmw_winsys_screen *
vmw_winsys_create( int fd )
{
struct vmw_winsys_screen *vws;
- struct stat stat_buf;
-
- if (dev_hash == NULL) {
- dev_hash = util_hash_table_create(vmw_dev_hash, vmw_dev_compare);
- if (dev_hash == NULL)
- return NULL;
- }
-
- if (fstat(fd, &stat_buf))
- return NULL;
-
- vws = util_hash_table_get(dev_hash, &stat_buf.st_rdev);
- if (vws) {
- vws->open_count++;
- return vws;
- }

vws = CALLOC_STRUCT(vmw_winsys_screen);
if (!vws)
goto out_no_vws;

- vws->device = stat_buf.st_rdev;
- vws->open_count = 1;
vws->ioctl.drm_fd = fcntl(fd, F_DUPFD_CLOEXEC, 3);
vws->base.have_gb_dma = TRUE;
vws->base.need_to_rebind_resources = FALSE;
@@ -106,14 +74,10 @@ vmw_winsys_create( int fd )
if (!vmw_winsys_screen_init_svga(vws))
goto out_no_svga;

- if (util_hash_table_set(dev_hash, &vws->device, vws) != PIPE_OK)
- goto out_no_hash_insert;
-
cnd_init(&vws->cs_cond);
mtx_init(&vws->cs_mutex, mtx_plain);

return vws;
-out_no_hash_insert:
out_no_svga:
vmw_pools_cleanup(vws);
out_no_pools:
@@ -130,14 +94,11 @@ out_no_vws:
void
vmw_winsys_destroy(struct vmw_winsys_screen *vws)
{
- if (--vws->open_count == 0) {
- util_hash_table_remove(dev_hash, &vws->device);
- vmw_pools_cleanup(vws);
- vws->fence_ops->destroy(vws->fence_ops);
- vmw_ioctl_cleanup(vws);
- close(vws->ioctl.drm_fd);
- mtx_destroy(&vws->cs_mutex);
- cnd_destroy(&vws->cs_cond);
- FREE(vws);
- }
+ vmw_pools_cleanup(vws);
+ vws->fence_ops->destroy(vws->fence_ops);
+ vmw_ioctl_cleanup(vws);
+ close(vws->ioctl.drm_fd);
+ mtx_destroy(&vws->cs_mutex);
+ cnd_destroy(&vws->cs_cond);
+ FREE(vws);
}
diff --git a/src/gallium/winsys/svga/drm/vmw_screen.h b/src/gallium/winsys/svga/drm/vmw_screen.h
index f21cabb51f93..e5faf36e73dc 100644
--- a/src/gallium/winsys/svga/drm/vmw_screen.h
+++ b/src/gallium/winsys/svga/drm/vmw_screen.h
@@ -94,12 +94,6 @@ struct vmw_winsys_screen

struct pb_fence_ops *fence_ops;

- /*
- * Screen instances
- */
- dev_t device;
- int open_count;
-
cnd_t cs_cond;
mtx_t cs_mutex;
};
--
2.11.0
Rob Herring
2017-08-07 22:58:17 UTC
Reply
Permalink
Raw Message
Use the common pipe_screen ref counting and fd hashing functions for
vc4. This is necessary to only create a single pipe_screen for a
process and avoid multiple imports of same prime fd.

Cc: Eric Anholt <***@anholt.net>
Signed-off-by: Rob Herring <***@kernel.org>
---
src/gallium/winsys/vc4/drm/vc4_drm_winsys.c | 21 +++++++++++++++++++--
1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/src/gallium/winsys/vc4/drm/vc4_drm_winsys.c b/src/gallium/winsys/vc4/drm/vc4_drm_winsys.c
index b2ffa90fe19e..51c6414ad848 100644
--- a/src/gallium/winsys/vc4/drm/vc4_drm_winsys.c
+++ b/src/gallium/winsys/vc4/drm/vc4_drm_winsys.c
@@ -27,15 +27,32 @@
#include "renderonly/renderonly.h"
#include "vc4_drm_public.h"
#include "vc4/vc4_screen.h"
+#include "util/u_screen.h"

struct pipe_screen *
vc4_drm_screen_create(int fd)
{
- return vc4_screen_create(fcntl(fd, F_DUPFD_CLOEXEC, 3), NULL);
+ int dupfd;
+ struct pipe_screen *pscreen = pipe_screen_reference(fd);
+ if (pscreen)
+ return pscreen;
+
+ dupfd = fcntl(fd, F_DUPFD_CLOEXEC, 3);
+ pscreen = vc4_screen_create(dupfd, NULL);
+ pipe_screen_reference_init(pscreen, dupfd);
+ return pscreen;
}

struct pipe_screen *
vc4_drm_screen_create_renderonly(struct renderonly *ro)
{
- return vc4_screen_create(fcntl(ro->gpu_fd, F_DUPFD_CLOEXEC, 3), ro);
+ int dupfd;
+ struct pipe_screen *pscreen = pipe_screen_reference(ro->gpu_fd);
+ if (pscreen)
+ return pscreen;
+
+ dupfd = fcntl(ro->gpu_fd, F_DUPFD_CLOEXEC, 3);
+ pscreen = vc4_screen_create(dupfd, ro);
+ pipe_screen_reference_init(pscreen, dupfd);
+ return pscreen;
}
--
2.11.0
Rob Herring
2017-08-07 22:58:18 UTC
Reply
Permalink
Raw Message
Use the common pipe_screen ref counting and fd hashing functions.
The mutex can be dropped as the pipe loader serializes the
create_screen() and destroy() calls.

Signed-off-by: Rob Herring <***@kernel.org>
Cc: Dave Airlie <***@redhat.com>
---
src/gallium/winsys/virgl/drm/virgl_drm_winsys.c | 88 +++----------------------
1 file changed, 10 insertions(+), 78 deletions(-)

diff --git a/src/gallium/winsys/virgl/drm/virgl_drm_winsys.c b/src/gallium/winsys/virgl/drm/virgl_drm_winsys.c
index 7f542e7f1ff4..c6795afb0698 100644
--- a/src/gallium/winsys/virgl/drm/virgl_drm_winsys.c
+++ b/src/gallium/winsys/virgl/drm/virgl_drm_winsys.c
@@ -25,7 +25,6 @@
#include <fcntl.h>
#include <stdio.h>
#include <sys/ioctl.h>
-#include <sys/stat.h>

#include "os/os_mman.h"
#include "os/os_time.h"
@@ -33,6 +32,7 @@
#include "util/u_format.h"
#include "util/u_hash_table.h"
#include "util/u_inlines.h"
+#include "util/u_screen.h"
#include "state_tracker/drm_driver.h"
#include "virgl/virgl_screen.h"
#include "virgl/virgl_public.h"
@@ -815,86 +815,18 @@ virgl_drm_winsys_create(int drmFD)

}

-static struct util_hash_table *fd_tab = NULL;
-static mtx_t virgl_screen_mutex = _MTX_INITIALIZER_NP;
-
-static void
-virgl_drm_screen_destroy(struct pipe_screen *pscreen)
-{
- struct virgl_screen *screen = virgl_screen(pscreen);
- boolean destroy;
-
- mtx_lock(&virgl_screen_mutex);
- destroy = --screen->refcnt == 0;
- if (destroy) {
- int fd = virgl_drm_winsys(screen->vws)->fd;
- util_hash_table_remove(fd_tab, intptr_to_pointer(fd));
- }
- mtx_unlock(&virgl_screen_mutex);
-
- if (destroy) {
- pscreen->destroy = screen->winsys_priv;
- pscreen->destroy(pscreen);
- }
-}
-
-static unsigned hash_fd(void *key)
-{
- int fd = pointer_to_intptr(key);
- struct stat stat;
- fstat(fd, &stat);
-
- return stat.st_dev ^ stat.st_ino ^ stat.st_rdev;
-}
-
-static int compare_fd(void *key1, void *key2)
-{
- int fd1 = pointer_to_intptr(key1);
- int fd2 = pointer_to_intptr(key2);
- struct stat stat1, stat2;
- fstat(fd1, &stat1);
- fstat(fd2, &stat2);
-
- return stat1.st_dev != stat2.st_dev ||
- stat1.st_ino != stat2.st_ino ||
- stat1.st_rdev != stat2.st_rdev;
-}
-
struct pipe_screen *
virgl_drm_screen_create(int fd)
{
- struct pipe_screen *pscreen = NULL;
-
- mtx_lock(&virgl_screen_mutex);
- if (!fd_tab) {
- fd_tab = util_hash_table_create(hash_fd, compare_fd);
- if (!fd_tab)
- goto unlock;
- }
-
- pscreen = util_hash_table_get(fd_tab, intptr_to_pointer(fd));
- if (pscreen) {
- virgl_screen(pscreen)->refcnt++;
- } else {
- struct virgl_winsys *vws;
- int dup_fd = fcntl(fd, F_DUPFD_CLOEXEC, 3);
-
- vws = virgl_drm_winsys_create(dup_fd);
-
- pscreen = virgl_create_screen(vws);
- if (pscreen) {
- util_hash_table_set(fd_tab, intptr_to_pointer(dup_fd), pscreen);
-
- /* Bit of a hack, to avoid circular linkage dependency,
- * ie. pipe driver having to call in to winsys, we
- * override the pipe drivers screen->destroy():
- */
- virgl_screen(pscreen)->winsys_priv = pscreen->destroy;
- pscreen->destroy = virgl_drm_screen_destroy;
- }
- }
+ int dupfd;
+ struct virgl_winsys *vws;
+ struct pipe_screen *pscreen = pipe_screen_reference(fd);
+ if (pscreen)
+ return pscreen;

-unlock:
- mtx_unlock(&virgl_screen_mutex);
+ dupfd = fcntl(fd, F_DUPFD_CLOEXEC, 3);
+ vws = virgl_drm_winsys_create(dupfd);
+ pscreen = virgl_create_screen(vws);
+ pipe_screen_reference_init(pscreen, dupfd);
return pscreen;
}
--
2.11.0
Rob Herring
2017-08-07 22:58:19 UTC
Reply
Permalink
Raw Message
Use the common pipe_screen ref counting and fd hashing functions for
vc4. This is necessary to only create a single pipe_screen for a
process and avoid multiple imports of same prime fd.

Cc: Eric Anholt <***@anholt.net>
Signed-off-by: Rob Herring <***@kernel.org>
---
src/gallium/winsys/vc4/drm/vc4_drm_winsys.c | 21 +++++++++++++++++++--
1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/src/gallium/winsys/vc4/drm/vc4_drm_winsys.c b/src/gallium/winsys/vc4/drm/vc4_drm_winsys.c
index b2ffa90fe19e..51c6414ad848 100644
--- a/src/gallium/winsys/vc4/drm/vc4_drm_winsys.c
+++ b/src/gallium/winsys/vc4/drm/vc4_drm_winsys.c
@@ -27,15 +27,32 @@
#include "renderonly/renderonly.h"
#include "vc4_drm_public.h"
#include "vc4/vc4_screen.h"
+#include "util/u_screen.h"

struct pipe_screen *
vc4_drm_screen_create(int fd)
{
- return vc4_screen_create(fcntl(fd, F_DUPFD_CLOEXEC, 3), NULL);
+ int dupfd;
+ struct pipe_screen *pscreen = pipe_screen_reference(fd);
+ if (pscreen)
+ return pscreen;
+
+ dupfd = fcntl(fd, F_DUPFD_CLOEXEC, 3);
+ pscreen = vc4_screen_create(dupfd, NULL);
+ pipe_screen_reference_init(pscreen, dupfd);
+ return pscreen;
}

struct pipe_screen *
vc4_drm_screen_create_renderonly(struct renderonly *ro)
{
- return vc4_screen_create(fcntl(ro->gpu_fd, F_DUPFD_CLOEXEC, 3), ro);
+ int dupfd;
+ struct pipe_screen *pscreen = pipe_screen_reference(ro->gpu_fd);
+ if (pscreen)
+ return pscreen;
+
+ dupfd = fcntl(ro->gpu_fd, F_DUPFD_CLOEXEC, 3);
+ pscreen = vc4_screen_create(dupfd, ro);
+ pipe_screen_reference_init(pscreen, dupfd);
+ return pscreen;
}
--
2.11.0
Loading...