[Mesa-dev] [PATCH 7/7] radeon/winsys: keep screen pointer in winsys

Mon Sep 23 07:52:57 PDT 2013

This is kind of hard to read. I think it would be nicer to:
1) Move the pipe_reference variable from the winsys to the screen.
2) Have the screen create the winsys, instead of just receiving a
pointer to it, so that the winsys doesn't have to be
reference-counted, because the screen is.

Marek

On Mon, Sep 23, 2013 at 3:58 PM, Christian König
<deathsimple at vodafone.de> wrote:
> From: Christian König <christian.koenig at amd.com>
>
> Only create one screen for each winsys instance.
> This helps with buffer sharing and interop handling.
>
> Signed-off-by: Christian König <christian.koenig at amd.com>
> ---
>  src/gallium/drivers/r300/r300_screen.c            |  3 +++
>  src/gallium/drivers/r600/r600_pipe.c              |  3 +++
>  src/gallium/drivers/radeonsi/radeonsi_pipe.c      |  3 +++
>  src/gallium/targets/dri-r300/target.c             | 13 ++++++++-----
>  src/gallium/targets/dri-r600/target.c             | 13 ++++++++-----
>  src/gallium/targets/dri-radeonsi/target.c         | 13 ++++++++-----
>  src/gallium/targets/vdpau-r300/target.c           | 13 ++++++++-----
>  src/gallium/targets/vdpau-r600/target.c           | 13 ++++++++-----
>  src/gallium/targets/vdpau-radeonsi/target.c       | 13 ++++++++-----
>  src/gallium/targets/xorg-r600/target.c            | 13 ++++++++-----
>  src/gallium/targets/xorg-radeonsi/target.c        | 13 ++++++++-----
>  src/gallium/targets/xvmc-r300/target.c            | 13 ++++++++-----
>  src/gallium/targets/xvmc-r600/target.c            | 13 ++++++++-----
>  src/gallium/winsys/radeon/drm/radeon_drm_winsys.c |  4 ----
>  src/gallium/winsys/radeon/drm/radeon_winsys.h     |  5 +++++
>  15 files changed, 94 insertions(+), 54 deletions(-)
>
> diff --git a/src/gallium/drivers/r300/r300_screen.c b/src/gallium/drivers/r300/r300_screen.c
> index 063bc09..45a815a 100644
> --- a/src/gallium/drivers/r300/r300_screen.c
> +++ b/src/gallium/drivers/r300/r300_screen.c
> @@ -540,6 +540,9 @@ 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 && !pipe_reference(&rws->reference, NULL))
> +      return;
> +
>      pipe_mutex_destroy(r300screen->cmask_mutex);
>
>      if (rws)
> diff --git a/src/gallium/drivers/r600/r600_pipe.c b/src/gallium/drivers/r600/r600_pipe.c
> index 50ff06c..4dee37b 100644
> --- a/src/gallium/drivers/r600/r600_pipe.c
> +++ b/src/gallium/drivers/r600/r600_pipe.c
> @@ -958,6 +958,9 @@ static void r600_destroy_screen(struct pipe_screen* pscreen)
>         if (rscreen == NULL)
>                 return;
>
> +       if (!pipe_reference(&rscreen->b.ws->reference, NULL))
> +               return;
> +
>         pipe_mutex_destroy(rscreen->aux_context_lock);
>         rscreen->aux_context->destroy(rscreen->aux_context);
>
> diff --git a/src/gallium/drivers/radeonsi/radeonsi_pipe.c b/src/gallium/drivers/radeonsi/radeonsi_pipe.c
> index 138268c..8529380 100644
> --- a/src/gallium/drivers/radeonsi/radeonsi_pipe.c
> +++ b/src/gallium/drivers/radeonsi/radeonsi_pipe.c
> @@ -648,6 +648,9 @@ static void r600_destroy_screen(struct pipe_screen* pscreen)
>         if (rscreen == NULL)
>                 return;
>
> +       if (!pipe_reference(&rscreen->b.ws->reference, NULL))
> +               return;
> +
>         if (rscreen->fences.bo) {
>                 struct r600_fence_block *entry, *tmp;
>
> diff --git a/src/gallium/targets/dri-r300/target.c b/src/gallium/targets/dri-r300/target.c
> index 07b0705..d363885 100644
> --- a/src/gallium/targets/dri-r300/target.c
> +++ b/src/gallium/targets/dri-r300/target.c
> @@ -1,25 +1,28 @@
>  #include "target-helpers/inline_debug_helper.h"
>  #include "state_tracker/drm_driver.h"
>  #include "radeon/drm/radeon_drm_public.h"
> +#include "radeon/drm/radeon_winsys.h"
>  #include "r300/r300_public.h"
>
>  static struct pipe_screen *
>  create_screen(int fd)
>  {
>     struct radeon_winsys *sws;
> -   struct pipe_screen *screen;
>
>     sws = radeon_drm_winsys_create(fd);
>     if (!sws)
>        return NULL;
>
> -   screen = r300_screen_create(sws);
> -   if (!screen)
> +   if (sws->screen)
> +      return sws->screen;
> +
> +   sws->screen = r300_screen_create(sws);
> +   if (!sws->screen)
>        return NULL;
>
> -   screen = debug_screen_wrap(screen);
> +   sws->screen = debug_screen_wrap(sws->screen);
>
> -   return screen;
> +   return sws->screen;
>  }
>
>  DRM_DRIVER_DESCRIPTOR("r300", "radeon", create_screen, NULL)
> diff --git a/src/gallium/targets/dri-r600/target.c b/src/gallium/targets/dri-r600/target.c
> index 9bff6f2..3fc4422 100644
> --- a/src/gallium/targets/dri-r600/target.c
> +++ b/src/gallium/targets/dri-r600/target.c
> @@ -1,24 +1,27 @@
>  #include "state_tracker/drm_driver.h"
>  #include "target-helpers/inline_debug_helper.h"
>  #include "radeon/drm/radeon_drm_public.h"
> +#include "radeon/drm/radeon_winsys.h"
>  #include "r600/r600_public.h"
>
>  static struct pipe_screen *create_screen(int fd)
>  {
>     struct radeon_winsys *radeon;
> -   struct pipe_screen *screen;
>
>     radeon = radeon_drm_winsys_create(fd);
>     if (!radeon)
>        return NULL;
>
> -   screen = r600_screen_create(radeon);
> -   if (!screen)
> +   if (radeon->screen)
> +      return radeon->screen;
> +
> +   radeon->screen = r600_screen_create(radeon);
> +   if (!radeon->screen)
>        return NULL;
>
> -   screen = debug_screen_wrap(screen);
> +   radeon->screen = debug_screen_wrap(radeon->screen);
>
> -   return screen;
> +   return radeon->screen;
>  }
>
>  static const struct drm_conf_ret throttle_ret = {
> diff --git a/src/gallium/targets/dri-radeonsi/target.c b/src/gallium/targets/dri-radeonsi/target.c
> index 1350ba2..a294633 100644
> --- a/src/gallium/targets/dri-radeonsi/target.c
> +++ b/src/gallium/targets/dri-radeonsi/target.c
> @@ -1,24 +1,27 @@
>  #include "state_tracker/drm_driver.h"
>  #include "target-helpers/inline_debug_helper.h"
>  #include "radeon/drm/radeon_drm_public.h"
> +#include "radeon/drm/radeon_winsys.h"
>  #include "radeonsi/radeonsi_public.h"
>
>  static struct pipe_screen *create_screen(int fd)
>  {
>     struct radeon_winsys *radeon;
> -   struct pipe_screen *screen;
>
>     radeon = radeon_drm_winsys_create(fd);
>     if (!radeon)
>        return NULL;
>
> -   screen = radeonsi_screen_create(radeon);
> -   if (!screen)
> +   if (radeon->screen)
> +      return radeon->screen;
> +
> +   radeon->screen = radeonsi_screen_create(radeon);
> +   if (!radeon->screen)
>        return NULL;
>
> -   screen = debug_screen_wrap(screen);
> +   radeon->screen = debug_screen_wrap(radeon->screen);
>
> -   return screen;
> +   return radeon->screen;
>  }
>
>  static const struct drm_conf_ret throttle_ret = {
> diff --git a/src/gallium/targets/vdpau-r300/target.c b/src/gallium/targets/vdpau-r300/target.c
> index 2fd7c2f..042e080 100644
> --- a/src/gallium/targets/vdpau-r300/target.c
> +++ b/src/gallium/targets/vdpau-r300/target.c
> @@ -1,24 +1,27 @@
>  #include "state_tracker/drm_driver.h"
>  #include "target-helpers/inline_debug_helper.h"
>  #include "radeon/drm/radeon_drm_public.h"
> +#include "radeon/drm/radeon_winsys.h"
>  #include "r300/r300_public.h"
>
>  static struct pipe_screen *create_screen(int fd)
>  {
>     struct radeon_winsys *radeon;
> -   struct pipe_screen *screen;
>
>     radeon = radeon_drm_winsys_create(fd);
>     if (!radeon)
>        return NULL;
>
> -   screen = r300_screen_create(radeon);
> -   if (!screen)
> +   if (radeon->screen)
> +      return radeon->screen;
> +
> +   radeon->screen = r300_screen_create(radeon);
> +   if (!radeon->screen)
>        return NULL;
>
> -   screen = debug_screen_wrap(screen);
> +   radeon->screen = debug_screen_wrap(radeon->screen);
>
> -   return screen;
> +   return radeon->screen;
>  }
>
>  DRM_DRIVER_DESCRIPTOR("r300", "radeon", create_screen, NULL)
> diff --git a/src/gallium/targets/vdpau-r600/target.c b/src/gallium/targets/vdpau-r600/target.c
> index 3b7795b..b51c6af 100644
> --- a/src/gallium/targets/vdpau-r600/target.c
> +++ b/src/gallium/targets/vdpau-r600/target.c
> @@ -1,24 +1,27 @@
>  #include "state_tracker/drm_driver.h"
>  #include "target-helpers/inline_debug_helper.h"
>  #include "radeon/drm/radeon_drm_public.h"
> +#include "radeon/drm/radeon_winsys.h"
>  #include "r600/r600_public.h"
>
>  static struct pipe_screen *create_screen(int fd)
>  {
>     struct radeon_winsys *radeon;
> -   struct pipe_screen *screen;
>
>     radeon = radeon_drm_winsys_create(fd);
>     if (!radeon)
>        return NULL;
>
> -   screen = r600_screen_create(radeon);
> -   if (!screen)
> +   if (radeon->screen)
> +      return radeon->screen;
> +
> +   radeon->screen = r600_screen_create(radeon);
> +   if (!radeon->screen)
>        return NULL;
>
> -   screen = debug_screen_wrap(screen);
> +   radeon->screen = debug_screen_wrap(radeon->screen);
>
> -   return screen;
> +   return radeon->screen;
>  }
>
>  DRM_DRIVER_DESCRIPTOR("r600", "radeon", create_screen, NULL)
> diff --git a/src/gallium/targets/vdpau-radeonsi/target.c b/src/gallium/targets/vdpau-radeonsi/target.c
> index ffb6662..4d0b9e9 100644
> --- a/src/gallium/targets/vdpau-radeonsi/target.c
> +++ b/src/gallium/targets/vdpau-radeonsi/target.c
> @@ -1,24 +1,27 @@
>  #include "state_tracker/drm_driver.h"
>  #include "target-helpers/inline_debug_helper.h"
>  #include "radeon/drm/radeon_drm_public.h"
> +#include "radeon/drm/radeon_winsys.h"
>  #include "radeonsi/radeonsi_public.h"
>
>  static struct pipe_screen *create_screen(int fd)
>  {
>     struct radeon_winsys *radeon;
> -   struct pipe_screen *screen;
>
>     radeon = radeon_drm_winsys_create(fd);
>     if (!radeon)
>        return NULL;
>
> -   screen = radeonsi_screen_create(radeon);
> -   if (!screen)
> +   if (radeon->screen)
> +      return radeon->screen;
> +
> +   radeon->screen = radeonsi_screen_create(radeon);
> +   if (!radeon->screen)
>        return NULL;
>
> -   screen = debug_screen_wrap(screen);
> +   radeon->screen = debug_screen_wrap(radeon->screen);
>
> -   return screen;
> +   return radeon->screen;
>  }
>
>  DRM_DRIVER_DESCRIPTOR("radeonsi", "radeon", create_screen, NULL)
> diff --git a/src/gallium/targets/xorg-r600/target.c b/src/gallium/targets/xorg-r600/target.c
> index 75785da..81b5488 100644
> --- a/src/gallium/targets/xorg-r600/target.c
> +++ b/src/gallium/targets/xorg-r600/target.c
> @@ -2,25 +2,28 @@
>  #include "target-helpers/inline_debug_helper.h"
>  #include "state_tracker/drm_driver.h"
>  #include "radeon/drm/radeon_drm_public.h"
> +#include "radeon/drm/radeon_winsys.h"
>  #include "r600/r600_public.h"
>
>  static struct pipe_screen *
>  create_screen(int fd)
>  {
>     struct radeon_winsys *sws;
> -   struct pipe_screen *screen;
>
>     sws = radeon_drm_winsys_create(fd);
>     if (!sws)
>        return NULL;
>
> -   screen = r600_screen_create(sws);
> -   if (!screen)
> +   if (sws->screen)
> +      return sws->screen;
> +
> +   sws->screen = r600_screen_create(sws);
> +   if (!sws->screen)
>        return NULL;
>
> -   screen = debug_screen_wrap(screen);
> +   sws->screen = debug_screen_wrap(sws->screen);
>
> -   return screen;
> +   return sws->screen;
>  }
>
>  DRM_DRIVER_DESCRIPTOR("r600", "radeon", create_screen, NULL)
> diff --git a/src/gallium/targets/xorg-radeonsi/target.c b/src/gallium/targets/xorg-radeonsi/target.c
> index c023c68..6e618ad 100644
> --- a/src/gallium/targets/xorg-radeonsi/target.c
> +++ b/src/gallium/targets/xorg-radeonsi/target.c
> @@ -2,25 +2,28 @@
>  #include "target-helpers/inline_debug_helper.h"
>  #include "state_tracker/drm_driver.h"
>  #include "radeon/drm/radeon_drm_public.h"
> +#include "radeon/drm/radeon_winsys.h"
>  #include "radeonsi/radeonsi_public.h"
>
>  static struct pipe_screen *
>  create_screen(int fd)
>  {
>     struct radeon_winsys *sws;
> -   struct pipe_screen *screen;
>
>     sws = radeon_drm_winsys_create(fd);
>     if (!sws)
>        return NULL;
>
> -   screen = radeonsi_screen_create(sws);
> -   if (!screen)
> +   if (sws->screen)
> +      return sws->screen;
> +
> +   sws->screen = radeonsi_screen_create(sws);
> +   if (!sws->screen)
>        return NULL;
>
> -   screen = debug_screen_wrap(screen);
> +   sws->screen = debug_screen_wrap(sws->screen);
>
> -   return screen;
> +   return sws->screen;
>  }
>
>  DRM_DRIVER_DESCRIPTOR("radeonsi", "radeon", create_screen, NULL)
> diff --git a/src/gallium/targets/xvmc-r300/target.c b/src/gallium/targets/xvmc-r300/target.c
> index 2fd7c2f..042e080 100644
> --- a/src/gallium/targets/xvmc-r300/target.c
> +++ b/src/gallium/targets/xvmc-r300/target.c
> @@ -1,24 +1,27 @@
>  #include "state_tracker/drm_driver.h"
>  #include "target-helpers/inline_debug_helper.h"
>  #include "radeon/drm/radeon_drm_public.h"
> +#include "radeon/drm/radeon_winsys.h"
>  #include "r300/r300_public.h"
>
>  static struct pipe_screen *create_screen(int fd)
>  {
>     struct radeon_winsys *radeon;
> -   struct pipe_screen *screen;
>
>     radeon = radeon_drm_winsys_create(fd);
>     if (!radeon)
>        return NULL;
>
> -   screen = r300_screen_create(radeon);
> -   if (!screen)
> +   if (radeon->screen)
> +      return radeon->screen;
> +
> +   radeon->screen = r300_screen_create(radeon);
> +   if (!radeon->screen)
>        return NULL;
>
> -   screen = debug_screen_wrap(screen);
> +   radeon->screen = debug_screen_wrap(radeon->screen);
>
> -   return screen;
> +   return radeon->screen;
>  }
>
>  DRM_DRIVER_DESCRIPTOR("r300", "radeon", create_screen, NULL)
> diff --git a/src/gallium/targets/xvmc-r600/target.c b/src/gallium/targets/xvmc-r600/target.c
> index 3b7795b..b51c6af 100644
> --- a/src/gallium/targets/xvmc-r600/target.c
> +++ b/src/gallium/targets/xvmc-r600/target.c
> @@ -1,24 +1,27 @@
>  #include "state_tracker/drm_driver.h"
>  #include "target-helpers/inline_debug_helper.h"
>  #include "radeon/drm/radeon_drm_public.h"
> +#include "radeon/drm/radeon_winsys.h"
>  #include "r600/r600_public.h"
>
>  static struct pipe_screen *create_screen(int fd)
>  {
>     struct radeon_winsys *radeon;
> -   struct pipe_screen *screen;
>
>     radeon = radeon_drm_winsys_create(fd);
>     if (!radeon)
>        return NULL;
>
> -   screen = r600_screen_create(radeon);
> -   if (!screen)
> +   if (radeon->screen)
> +      return radeon->screen;
> +
> +   radeon->screen = r600_screen_create(radeon);
> +   if (!radeon->screen)
>        return NULL;
>
> -   screen = debug_screen_wrap(screen);
> +   radeon->screen = debug_screen_wrap(radeon->screen);
>
> -   return screen;
> +   return radeon->screen;
>  }
>
>  DRM_DRIVER_DESCRIPTOR("r600", "radeon", create_screen, NULL)
> diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
> index 61f0913..4f43093 100644
> --- a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
> +++ b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
> @@ -424,10 +424,6 @@ static void radeon_winsys_destroy(struct radeon_winsys *rws)
>  {
>      struct radeon_drm_winsys *ws = (struct radeon_drm_winsys*)rws;
>
> -    if (!pipe_reference(&ws->base.reference, NULL)) {
> -        return;
> -    }
> -
>      if (ws->thread) {
>          ws->kill_thread = 1;
>          pipe_semaphore_signal(&ws->cs_queued);
> diff --git a/src/gallium/winsys/radeon/drm/radeon_winsys.h b/src/gallium/winsys/radeon/drm/radeon_winsys.h
> index 1367982..7ed2437 100644
> --- a/src/gallium/winsys/radeon/drm/radeon_winsys.h
> +++ b/src/gallium/winsys/radeon/drm/radeon_winsys.h
> @@ -208,6 +208,11 @@ struct radeon_winsys {
>      struct pipe_reference reference;
>
>      /**
> +     * The screen object this winsys was created for
> +     */
> +    struct pipe_screen *screen;
> +
> +    /**
>       * Destroy this winsys.
>       *
>       * \param ws        The winsys this function is called from.
> --
> 1.8.1.2
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev