[Mesa-dev,v4,09/11] vmwgfx: use common screen ref counting

Submitted by Rob Herring on July 22, 2016, 4:22 p.m.

Details

Message ID 20160722162222.21871-10-robh@kernel.org
State New
Headers show
Series "Common pipe screen ref counting" ( rev: 4 ) in Mesa

Not browsing as part of any series.

Commit Message

Rob Herring July 22, 2016, 4:22 p.m.
Use the common pipe_screen ref counting and fd hashing functions. The
mutex can be dropped as the pipe loader protects the create_screen()
calls.

Signed-off-by: Rob Herring <robh@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            |  5 ++-
 src/gallium/targets/pipe-loader/pipe_vmwgfx.c     |  2 +-
 src/gallium/winsys/svga/drm/vmw_screen.c          | 54 +++++------------------
 src/gallium/winsys/svga/drm/vmw_screen.h          |  6 ---
 6 files changed, 17 insertions(+), 54 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/gallium/auxiliary/target-helpers/drm_helper.h b/src/gallium/auxiliary/target-helpers/drm_helper.h
index 90820d3..a042162 100644
--- a/src/gallium/auxiliary/target-helpers/drm_helper.h
+++ b/src/gallium/auxiliary/target-helpers/drm_helper.h
@@ -181,7 +181,7 @@  pipe_vmwgfx_create_screen(int fd)
    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 ded2e24..5a95660 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 5b4ac74..b353b92 100644
--- a/src/gallium/drivers/svga/svga_screen.c
+++ b/src/gallium/drivers/svga/svga_screen.c
@@ -26,6 +26,7 @@ 
 #include "util/u_format.h"
 #include "util/u_memory.h"
 #include "util/u_inlines.h"
+#include "util/u_screen.h"
 #include "util/u_string.h"
 #include "util/u_math.h"
 
@@ -906,7 +907,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;
@@ -1081,6 +1082,8 @@  svga_screen_create(struct svga_winsys_screen *sws)
 
    svga_screen_cache_init(svgascreen);
 
+   pipe_screen_reference_init(screen, dup(fd));
+
    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 71015df..d246022 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)
    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 7fcb6d2..e0fa763 100644
--- a/src/gallium/winsys/svga/drm/vmw_screen.c
+++ b/src/gallium/winsys/svga/drm/vmw_screen.c
@@ -29,25 +29,11 @@ 
 #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"
 #include <sys/types.h>
-#include <sys/stat.h>
 #include <unistd.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.
@@ -60,29 +46,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 = dup(fd);
    vws->base.have_gb_dma = TRUE;
    vws->base.need_to_rebind_resources = FALSE;
@@ -100,11 +72,8 @@  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;
-
    return vws;
-out_no_hash_insert:
+
 out_no_svga:
    vmw_pools_cleanup(vws);
 out_no_pools:
@@ -121,12 +90,9 @@  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);
-      FREE(vws);
-   }
+   vmw_pools_cleanup(vws);
+   vws->fence_ops->destroy(vws->fence_ops);
+   vmw_ioctl_cleanup(vws);
+   close(vws->ioctl.drm_fd);
+   FREE(vws);
 }
diff --git a/src/gallium/winsys/svga/drm/vmw_screen.h b/src/gallium/winsys/svga/drm/vmw_screen.h
index 79d0949..cfde6bb 100644
--- a/src/gallium/winsys/svga/drm/vmw_screen.h
+++ b/src/gallium/winsys/svga/drm/vmw_screen.h
@@ -93,12 +93,6 @@  struct vmw_winsys_screen
    } pools;
 
    struct pb_fence_ops *fence_ops;
-
-   /*
-    * Screen instances
-    */
-   dev_t device;
-   int open_count;
 };
 
 

Comments

On 07/22/2016 10:22 AM, Rob Herring wrote:
> Use the common pipe_screen ref counting and fd hashing functions. The
> mutex can be dropped as the pipe loader protects the create_screen()
> calls.
>
> Signed-off-by: Rob Herring <robh@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            |  5 ++-
>   src/gallium/targets/pipe-loader/pipe_vmwgfx.c     |  2 +-
>   src/gallium/winsys/svga/drm/vmw_screen.c          | 54 +++++------------------
>   src/gallium/winsys/svga/drm/vmw_screen.h          |  6 ---
>   6 files changed, 17 insertions(+), 54 deletions(-)
>
> diff --git a/src/gallium/auxiliary/target-helpers/drm_helper.h b/src/gallium/auxiliary/target-helpers/drm_helper.h
> index 90820d3..a042162 100644
> --- a/src/gallium/auxiliary/target-helpers/drm_helper.h
> +++ b/src/gallium/auxiliary/target-helpers/drm_helper.h
> @@ -181,7 +181,7 @@ pipe_vmwgfx_create_screen(int fd)
>      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 ded2e24..5a95660 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 5b4ac74..b353b92 100644
> --- a/src/gallium/drivers/svga/svga_screen.c
> +++ b/src/gallium/drivers/svga/svga_screen.c
> @@ -26,6 +26,7 @@
>   #include "util/u_format.h"
>   #include "util/u_memory.h"
>   #include "util/u_inlines.h"
> +#include "util/u_screen.h"
>   #include "util/u_string.h"
>   #include "util/u_math.h"
>
> @@ -906,7 +907,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;
> @@ -1081,6 +1082,8 @@ svga_screen_create(struct svga_winsys_screen *sws)
>
>      svga_screen_cache_init(svgascreen);
>
> +   pipe_screen_reference_init(screen, dup(fd));

dup() is not a Windows function.  I'm not sure where the prototype for 
dup() is getting pulled in on Linux either.  Maybe this file needs:

#ifdef PIPE_OS_WINDOWS
static int dup(int fd)
{
    return fd;
}
#else
#include <unistd.h>
#endif

Surprisingly, #include <sys/stat.h> in u_screen.c seems to compile with 
MSVC.

-Brian



> +
>      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 71015df..d246022 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)
>      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 7fcb6d2..e0fa763 100644
> --- a/src/gallium/winsys/svga/drm/vmw_screen.c
> +++ b/src/gallium/winsys/svga/drm/vmw_screen.c
> @@ -29,25 +29,11 @@
>   #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"
>   #include <sys/types.h>
> -#include <sys/stat.h>
>   #include <unistd.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.
> @@ -60,29 +46,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 = dup(fd);
>      vws->base.have_gb_dma = TRUE;
>      vws->base.need_to_rebind_resources = FALSE;
> @@ -100,11 +72,8 @@ 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;
> -
>      return vws;
> -out_no_hash_insert:
> +
>   out_no_svga:
>      vmw_pools_cleanup(vws);
>   out_no_pools:
> @@ -121,12 +90,9 @@ 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);
> -      FREE(vws);
> -   }
> +   vmw_pools_cleanup(vws);
> +   vws->fence_ops->destroy(vws->fence_ops);
> +   vmw_ioctl_cleanup(vws);
> +   close(vws->ioctl.drm_fd);
> +   FREE(vws);
>   }
> diff --git a/src/gallium/winsys/svga/drm/vmw_screen.h b/src/gallium/winsys/svga/drm/vmw_screen.h
> index 79d0949..cfde6bb 100644
> --- a/src/gallium/winsys/svga/drm/vmw_screen.h
> +++ b/src/gallium/winsys/svga/drm/vmw_screen.h
> @@ -93,12 +93,6 @@ struct vmw_winsys_screen
>      } pools;
>
>      struct pb_fence_ops *fence_ops;
> -
> -   /*
> -    * Screen instances
> -    */
> -   dev_t device;
> -   int open_count;
>   };
>
>
>
On 22 July 2016 at 17:22, Rob Herring <robh@kernel.org> wrote:

> @@ -100,11 +72,8 @@ 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;
> -
Any reason why you didn't call pipe_screen_reference_init() here but
you've moved in the driver ?

-Emil
On 26 July 2016 at 10:59, Emil Velikov <emil.l.velikov@gmail.com> wrote:
> On 22 July 2016 at 17:22, Rob Herring <robh@kernel.org> wrote:
>
>> @@ -100,11 +72,8 @@ 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;
>> -
> Any reason why you didn't call pipe_screen_reference_init() here but
> you've moved in the driver ?
>
Grr just hit me - because unlike other drivers svga does not call
screen_create in it's winsys. Other drivers did that to reuse the
locking for both fd and screen management, but with the separate locks
introduced in the series things should be great.

In theory one could 'decouple' the radeon/amdgpu/nouvea... winsys and
drivers, just like svga. If they're interested/see value in it.

Thanks
Emil