Re: [Mesa-dev] [PATCH v5 09/12] radeon: use common pipe_screen ref counting

[Marek Olšák]

On Wed, Aug 9, 2017 at 10:34 PM, Rob Herring  wrote:
> On Wed, Aug 9, 2017 at 3:19 PM, Marek Olšák  wrote:
>> 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 

Marek
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v5 09/12] radeon: use common pipe_screen ref counting

[Rob Herring]

On Wed, Aug 9, 2017 at 3:19 PM, Marek Olšák  wrote:
> You already know I can't accept the amdgpu part.

You don't agree with my explanation in the other patch?

> 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
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v5 09/12] radeon: use common pipe_screen ref counting

[Marek Olšák]

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

On Tue, Aug 8, 2017 at 12:58 AM, Rob Herring  wrote:
> 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 
> Cc: "Marek Olšák" 
> Cc: Ilia Mirkin 
> ---
>  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(>cmask_mutex);
>  slab_destroy_parent(>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.
> - * \returnTrue if the winsys and screen should be destroyed.
> - */
> -bool (*unref)(struct radeon_winsys *ws);
> -
> -/**
>   * Destroy this winsys.
>   *
>   * \param wsThe 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(>shader_compiler_queue);
> util_queue_destroy(>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