[Mesa-dev] [PATCH 1/2] virgl: reuse screen when fd is already open

[Emil Velikov]

Hi Rob,

On 30 January 2016 at 00:36, Rob Herring <robh at kernel.org> wrote:
> It is necessary to share the screen between mesa and gralloc to
> properly ref count resources. This implements a hash lookup on
> the file description to re-use an already created screen. This is
> a similar implementation as freedreno and radeon.
>
I believe you mentioned this before ... can we share this across drivers.

Taking that and going the extra step... I'm thinking about exporting
the private symbol. This way we can get rid of the
"foo_drm_create_screen" symbol that each platform needs to export (and
alike 'hack' for Android).

About the actual location - I'm leaning towards
src/gallium/auxiliary/target-helpers/foo.c although I don't feel too
strongly.

> --- a/src/gallium/drivers/virgl/virgl_screen.h
> +++ b/src/gallium/drivers/virgl/virgl_screen.h
> @@ -28,6 +28,12 @@
>
>  struct virgl_screen {
>     struct pipe_screen base;
> +
> +   int refcnt;
> +
> +   /* place for winsys to stash it's own stuff: */
> +   void *winsys_priv;
> +
In order to avoid this workaround (and similar ones like it) I'm
thinking about the following:
 - move refcnt to pipe_screen (use struct pipe_reference)
 - then within the foo_create_winsys we lock, create the actual
winsys, search for existing screen/create new one, refcount. unlock.

Afaict this approach is also applicable for
nouveau/radeon/amdgpu/nouveau although I'm not sure if people will
feel strongly against having the pipe_reference within pipe_screen.

> --- a/src/gallium/winsys/virgl/drm/virgl_drm_winsys.c
> +++ b/src/gallium/winsys/virgl/drm/virgl_drm_winsys.c

> @@ -772,3 +775,87 @@ virgl_drm_winsys_create(int drmFD)
>     return &qdws->base;
>
>  }
> +
> +static struct util_hash_table *fd_tab = NULL;
The first idea that comes to mind is to have something like

PUBLIC struct {
   struct util_hash_table *fd_tab;
   pipe_mutex mutex;
   int locked; // used for atomic check, toggle before the lock (if we
still need the lock that is)
} __fancyMesaDriverPrivateSymbol = { .fd_tab = NULL, .mutex =
_MTX_INITIALIZER_NP, .locked = 0 };

Rob, Christian, how does the idea sound ? As we one must keep
vdpau/dri modules in sync to avoid chaos things should be safe :-)
Plus it will also allow us to minimise the Android specific
workaround/hack.

-Emil
P.S. Obviously we'll need to update src/gallium/targets/{*/*.sym,dri-vdpau.dyn}